Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit8899dd8

Browse files
ammarioEmyrk
andauthored
chore: add global caching to rbac (#7439)
Co-authored-by: Steven Masley <stevenmasley@coder.com>
1 parent643a9ef commit8899dd8

File tree

13 files changed

+122
-126
lines changed

13 files changed

+122
-126
lines changed

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,3 +53,4 @@ site/stats/
5353

5454
# direnv
5555
.envrc
56+
*.test

‎.prettierignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ site/stats/
5656

5757
# direnv
5858
.envrc
59+
*.test
5960
# .prettierignore.include:
6061
# Helm templates contain variables that are invalid YAML and can't be formatted
6162
# by Prettier.

‎coderd/coderd.go‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,6 @@ func New(options *Options) *API {
398398
tracing.StatusWriterMiddleware,
399399
tracing.Middleware(api.TracerProvider),
400400
httpmw.AttachRequestID,
401-
httpmw.AttachAuthzCache,
402401
httpmw.ExtractRealIP(api.RealIPConfig),
403402
httpmw.Logger(api.Logger),
404403
httpmw.Prometheus(options.PrometheusRegistry),

‎coderd/coderdtest/coderdtest.go‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ type Options struct {
137137
}
138138

139139
// New constructs a codersdk client connected to an in-memory API instance.
140-
funcNew(t*testing.T,options*Options)*codersdk.Client {
140+
funcNew(t testing.TB,options*Options)*codersdk.Client {
141141
client,_:=newWithCloser(t,options)
142142
returnclient
143143
}
@@ -162,12 +162,12 @@ func NewWithProvisionerCloser(t *testing.T, options *Options) (*codersdk.Client,
162162
// upon thee. Even the io.Closer that is exposed here shouldn't be exposed
163163
// and is a temporary measure while the API to register provisioners is ironed
164164
// out.
165-
funcnewWithCloser(t*testing.T,options*Options) (*codersdk.Client, io.Closer) {
165+
funcnewWithCloser(t testing.TB,options*Options) (*codersdk.Client, io.Closer) {
166166
client,closer,_:=NewWithAPI(t,options)
167167
returnclient,closer
168168
}
169169

170-
funcNewOptions(t*testing.T,options*Options) (func(http.Handler), context.CancelFunc,*url.URL,*coderd.Options) {
170+
funcNewOptions(t testing.TB,options*Options) (func(http.Handler), context.CancelFunc,*url.URL,*coderd.Options) {
171171
ifoptions==nil {
172172
options=&Options{}
173173
}
@@ -190,8 +190,14 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
190190
}
191191

192192
ifoptions.Authorizer==nil {
193-
options.Authorizer=&RecordingAuthorizer{
194-
Wrapped:rbac.NewCachingAuthorizer(prometheus.NewRegistry()),
193+
defAuth:=rbac.NewCachingAuthorizer(prometheus.NewRegistry())
194+
if_,ok:=t.(*testing.T);ok {
195+
options.Authorizer=&RecordingAuthorizer{
196+
Wrapped:defAuth,
197+
}
198+
}else {
199+
// In benchmarks, the recording authorizer greatly skews results.
200+
options.Authorizer=defAuth
195201
}
196202
}
197203

@@ -359,7 +365,7 @@ func NewOptions(t *testing.T, options *Options) (func(http.Handler), context.Can
359365
// NewWithAPI constructs an in-memory API instance and returns a client to talk to it.
360366
// Most tests never need a reference to the API, but AuthorizationTest in this module uses it.
361367
// Do not expose the API or wrath shall descend upon thee.
362-
funcNewWithAPI(t*testing.T,options*Options) (*codersdk.Client, io.Closer,*coderd.API) {
368+
funcNewWithAPI(t testing.TB,options*Options) (*codersdk.Client, io.Closer,*coderd.API) {
363369
ifoptions==nil {
364370
options=&Options{}
365371
}
@@ -384,7 +390,7 @@ func NewWithAPI(t *testing.T, options *Options) (*codersdk.Client, io.Closer, *c
384390
// NewProvisionerDaemon launches a provisionerd instance configured to work
385391
// well with coderd testing. It registers the "echo" provisioner for
386392
// quick testing.
387-
funcNewProvisionerDaemon(t*testing.T,coderAPI*coderd.API) io.Closer {
393+
funcNewProvisionerDaemon(t testing.TB,coderAPI*coderd.API) io.Closer {
388394
echoClient,echoServer:=provisionersdk.MemTransportPipe()
389395
ctx,cancelFunc:=context.WithCancel(context.Background())
390396
t.Cleanup(func() {
@@ -465,7 +471,7 @@ var FirstUserParams = codersdk.CreateFirstUserRequest{
465471

466472
// CreateFirstUser creates a user with preset credentials and authenticates
467473
// with the passed in codersdk client.
468-
funcCreateFirstUser(t*testing.T,client*codersdk.Client) codersdk.CreateFirstUserResponse {
474+
funcCreateFirstUser(t testing.TB,client*codersdk.Client) codersdk.CreateFirstUserResponse {
469475
resp,err:=client.CreateFirstUser(context.Background(),FirstUserParams)
470476
require.NoError(t,err)
471477

@@ -1111,7 +1117,7 @@ sz9Di8sGIaUbLZI2rd0CQQCzlVwEtRtoNCyMJTTrkgUuNufLP19RZ5FpyXxBO5/u
11111117
QastnN77KfUwdj3SJt44U/uh1jAIv4oSLBr8HYUkbnI8
11121118
-----END RSA PRIVATE KEY-----`
11131119

1114-
funcDeploymentValues(t*testing.T)*codersdk.DeploymentValues {
1120+
funcDeploymentValues(t testing.TB)*codersdk.DeploymentValues {
11151121
varcfg codersdk.DeploymentValues
11161122
opts:=cfg.Options()
11171123
err:=opts.SetDefaults()

‎coderd/database/dbtestutil/db.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"github.com/coder/coder/coderd/database/postgres"
1414
)
1515

16-
funcNewDB(t*testing.T) (database.Store, database.Pubsub) {
16+
funcNewDB(t testing.TB) (database.Store, database.Pubsub) {
1717
t.Helper()
1818

1919
db:=dbfake.New()

‎coderd/httpmw/authz.go‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"github.com/go-chi/chi/v5"
77

88
"github.com/coder/coder/coderd/database/dbauthz"
9-
"github.com/coder/coder/coderd/rbac"
109
)
1110

1211
// AsAuthzSystem is a chained handler that temporarily sets the dbauthz context
@@ -36,16 +35,3 @@ func AsAuthzSystem(mws ...func(http.Handler) http.Handler) func(http.Handler) ht
3635
})
3736
}
3837
}
39-
40-
// AttachAuthzCache enables the authz cache for the authorizer. All rbac checks will
41-
// run against the cache, meaning duplicate checks will not be performed.
42-
//
43-
// Note the cache is safe for multiple actors. So mixing user and system checks
44-
// is ok.
45-
funcAttachAuthzCache(next http.Handler) http.Handler {
46-
returnhttp.HandlerFunc(func(rw http.ResponseWriter,r*http.Request) {
47-
ctx:=rbac.WithCacheCtx(r.Context())
48-
49-
next.ServeHTTP(rw,r.WithContext(ctx))
50-
})
51-
}

‎coderd/rbac/authz.go‎

Lines changed: 53 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,14 @@ package rbac
22

33
import (
44
"context"
5+
"crypto/sha256"
56
_"embed"
7+
"encoding/json"
68
"strings"
79
"sync"
810
"time"
911

12+
"github.com/ammario/tlru"
1013
"github.com/open-policy-agent/opa/ast"
1114
"github.com/open-policy-agent/opa/rego"
1215
"github.com/prometheus/client_golang/prometheus"
@@ -42,6 +45,31 @@ type AuthCall struct {
4245
ObjectObject
4346
}
4447

48+
// hashAuthorizeCall guarantees a unique hash for a given auth call.
49+
// If two hashes are equal, then the result of a given authorize() call
50+
// will be the same.
51+
//
52+
// Note that this ignores some fields such as the permissions within a given
53+
// role, as this assumes all roles are static to a given role name.
54+
funchashAuthorizeCall(actorSubject,actionAction,objectObject) [32]byte {
55+
varhashOut [32]byte
56+
hash:=sha256.New()
57+
58+
// We use JSON for the forward security benefits if the rbac structs are
59+
// modified without consideration for the caching layer.
60+
enc:=json.NewEncoder(hash)
61+
_=enc.Encode(actor)
62+
_=enc.Encode(action)
63+
_=enc.Encode(object)
64+
65+
// We might be able to avoid this extra copy?
66+
// sha256.Sum256() returns a [32]byte. We need to return
67+
// an array vs a slice so we can use it as a key in the cache.
68+
image:=hash.Sum(nil)
69+
copy(hashOut[:],image)
70+
returnhashOut
71+
}
72+
4573
// Subject is a struct that contains all the elements of a subject in an rbac
4674
// authorize.
4775
typeSubjectstruct {
@@ -101,6 +129,9 @@ func (s Subject) SafeRoleNames() []string {
101129
}
102130

103131
typeAuthorizerinterface {
132+
// Authorize will authorize the given subject to perform the given action
133+
// on the given object. Authorize is pure and deterministic with respect to
134+
// its arguments and the surrounding object.
104135
Authorize(ctx context.Context,subjectSubject,actionAction,objectObject)error
105136
Prepare(ctx context.Context,subjectSubject,actionAction,objectTypestring) (PreparedAuthorized,error)
106137
}
@@ -310,6 +341,7 @@ func (a RegoAuthorizer) Authorize(ctx context.Context, subject Subject, action A
310341
deferspan.End()
311342

312343
err:=a.authorize(ctx,subject,action,object)
344+
313345
span.SetAttributes(attribute.Bool("authorized",err==nil))
314346

315347
dur:=time.Since(start)
@@ -605,7 +637,12 @@ func (a *authorizedSQLFilter) SQLString() string {
605637
returna.sqlString
606638
}
607639

608-
typecachedCallsstruct {
640+
typeauthCachestruct {
641+
// cache is a cache of hashed Authorize inputs to the result of the Authorize
642+
// call.
643+
// determistic function.
644+
cache*tlru.Cache[[32]byte,error]
645+
609646
authzAuthorizer
610647
}
611648

@@ -617,94 +654,35 @@ type cachedCalls struct {
617654
//
618655
// Cacher is safe for multiple actors.
619656
funcCacher(authzAuthorizer)Authorizer {
620-
return&cachedCalls{authz:authz}
657+
return&authCache{
658+
authz:authz,
659+
// In practice, this cache should never come close to filling since the
660+
// authorization calls are kept for a minute at most.
661+
cache:tlru.New[[32]byte](tlru.ConstantCost[error],64*1024),
662+
}
621663
}
622664

623-
func (c*cachedCalls)Authorize(ctx context.Context,subjectSubject,actionAction,objectObject)error {
624-
cache:=cacheFromContext(ctx)
665+
func (c*authCache)Authorize(ctx context.Context,subjectSubject,actionAction,objectObject)error {
666+
authorizeCacheKey:=hashAuthorizeCall(subject,action,object)
625667

626-
resp,ok:=cache.Load(subject,action,object)
627-
ifok {
628-
returnresp
668+
varerrerror
669+
err,_,ok:=c.cache.Get(authorizeCacheKey)
670+
if!ok {
671+
err=c.authz.Authorize(ctx,subject,action,object)
672+
// In case there is a caching bug, bound the TTL to 1 minute.
673+
c.cache.Set(authorizeCacheKey,err,time.Minute)
629674
}
630675

631-
err:=c.authz.Authorize(ctx,subject,action,object)
632-
cache.Save(subject,action,object,err)
633676
returnerr
634677
}
635678

636679
// Prepare returns the underlying PreparedAuthorized. The cache does not apply
637680
// to prepared authorizations. These should be using a SQL filter, and
638681
// therefore the cache is not needed.
639-
func (c*cachedCalls)Prepare(ctx context.Context,subjectSubject,actionAction,objectTypestring) (PreparedAuthorized,error) {
682+
func (c*authCache)Prepare(ctx context.Context,subjectSubject,actionAction,objectTypestring) (PreparedAuthorized,error) {
640683
returnc.authz.Prepare(ctx,subject,action,objectType)
641684
}
642685

643-
// authorizeCache enabled caching of Authorizer calls for a given request. This
644-
// prevents the cost of running the same rbac checks multiple times.
645-
// A cache hit must match on all 3 values: subject, action, and object.
646-
typeauthorizeCachestruct {
647-
sync.Mutex
648-
// calls is a list of all calls made to the Authorizer.
649-
// This list is cached per request context. The size of this list is expected
650-
// to be incredibly small. Often 1 or 2 calls.
651-
calls []cachedAuthCall
652-
}
653-
654-
typecachedAuthCallstruct {
655-
AuthCall
656-
Errerror
657-
}
658-
659-
// cacheContextKey is a context key used to store the cache in the context.
660-
typecacheContextKeystruct{}
661-
662-
// cacheFromContext returns the cache from the context.
663-
// If there is no cache, a nil value is returned.
664-
// The nil cache can still be called as a normal cache, but will not cache or
665-
// return any values.
666-
funccacheFromContext(ctx context.Context)*authorizeCache {
667-
cache,_:=ctx.Value(cacheContextKey{}).(*authorizeCache)
668-
returncache
669-
}
670-
671-
funcWithCacheCtx(ctx context.Context) context.Context {
672-
returncontext.WithValue(ctx,cacheContextKey{},&authorizeCache{})
673-
}
674-
675-
//nolint:revive
676-
func (c*authorizeCache)Load(subjectSubject,actionAction,objectObject) (error,bool) {
677-
ifc==nil {
678-
returnnil,false
679-
}
680-
c.Lock()
681-
deferc.Unlock()
682-
683-
for_,call:=rangec.calls {
684-
ifcall.Action==action&&call.Object.Equal(object)&&call.Actor.Equal(subject) {
685-
returncall.Err,true
686-
}
687-
}
688-
returnnil,false
689-
}
690-
691-
func (c*authorizeCache)Save(subjectSubject,actionAction,objectObject,errerror) {
692-
ifc==nil {
693-
return
694-
}
695-
c.Lock()
696-
deferc.Unlock()
697-
698-
c.calls=append(c.calls,cachedAuthCall{
699-
AuthCall:AuthCall{
700-
Actor:subject,
701-
Action:action,
702-
Object:object,
703-
},
704-
Err:err,
705-
})
706-
}
707-
708686
// rbacTraceAttributes are the attributes that are added to all spans created by
709687
// the rbac package. These attributes should help to debug slow spans.
710688
funcrbacTraceAttributes(actorSubject,actionAction,objectTypestring,extra...attribute.KeyValue) trace.SpanStartOption {

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp