Skip to content

Commit 950d1ad

Browse files
committed
CheckBulkPermissions
Adds the 'PermissionsService/CheckBulkPermissions' API method. This promotes the existing 'ExperimentalService/BulkCheckPermission' API out of the experimental service, but does not remove the old method to ensure backwards compatibility. To reduce code duplication, this refactors the bulk permission checking logic into a shared component that can be used by both the old and new methods. The old method performs protobuf message conversion on the input, passes the inputs to the shared method, and then converts the output.
1 parent c82dc05 commit 950d1ad

16 files changed

+707
-287
lines changed

e2e/go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ go 1.21
55
toolchain go1.21.1
66

77
require (
8-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c
8+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d
99
github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1
1010
github.com/authzed/spicedb v1.29.1
1111
github.com/brianvoe/gofakeit/v6 v6.23.2

e2e/go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8
2020
github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10=
2121
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9 h1:goHVqTbFX3AIo0tzGr14pgfAW2ZfPChKO21Z9MGf/gk=
2222
github.com/antlr/antlr4/runtime/Go/antlr/v4 v4.0.0-20230512164433-5d1fd1a340c9/go.mod h1:pSwJ0fSY5KhvocuWSx4fz3BA8OrA1bQn+K1Eli3BRwM=
23-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c h1:w71KppS/+mCDkNXR8vmwXGVt/1dLt+axcIq+qK7f7To=
24-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c/go.mod h1:bS4eeTw/ZpCunZHePrt1MAcvOggAwL8Djh8cx+CR33g=
23+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d h1:hzo0UIaYtLyJsEdCrM05k1k2YZHqPAyMpHL7rq37j5Q=
24+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d/go.mod h1:2cnND+OBSxz1DpVfaQBKtdobTiaX2qAlj7k9eNr/pM4=
2525
github.com/authzed/cel-go v0.17.5 h1:lfpkNrR99B5QRHg5qdG9oLu/kguVlZC68VJuMk8tH9Y=
2626
github.com/authzed/cel-go v0.17.5/go.mod h1:XL/zEq5hKGVF8aOdMbG7w+BQPihLjY2W8N+UIygDA2I=
2727
github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1 h1:zBfQzia6Hz45pJBeURTrv1b6HezmejB6UmiGuBilHZM=

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ require (
88
contrib.go.opencensus.io/exporter/prometheus v0.4.2
99
github.com/IBM/pgxpoolprometheus v1.1.1
1010
github.com/Masterminds/squirrel v1.5.4
11-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c
11+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d
1212
github.com/authzed/cel-go v0.17.5
1313
github.com/authzed/consistent v0.1.0
1414
github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1

go.sum

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,8 @@ github.com/ashanbrown/forbidigo v1.6.0 h1:D3aewfM37Yb3pxHujIPSpTf6oQk9sc9WZi8ger
116116
github.com/ashanbrown/forbidigo v1.6.0/go.mod h1:Y8j9jy9ZYAEHXdu723cUlraTqbzjKF1MUyfOKL+AjcU=
117117
github.com/ashanbrown/makezero v1.1.1 h1:iCQ87C0V0vSyO+M9E/FZYbu65auqH0lnsOkf5FcB28s=
118118
github.com/ashanbrown/makezero v1.1.1/go.mod h1:i1bJLCRSCHOcOa9Y6MyF2FTfMZMFdHvxKHxgO5Z1axI=
119-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c h1:w71KppS/+mCDkNXR8vmwXGVt/1dLt+axcIq+qK7f7To=
120-
github.com/authzed/authzed-go v0.10.2-0.20240206183056-781a5f5d1b3c/go.mod h1:bS4eeTw/ZpCunZHePrt1MAcvOggAwL8Djh8cx+CR33g=
119+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d h1:hzo0UIaYtLyJsEdCrM05k1k2YZHqPAyMpHL7rq37j5Q=
120+
github.com/authzed/authzed-go v0.10.2-0.20240312180602-9b3947de5a3d/go.mod h1:2cnND+OBSxz1DpVfaQBKtdobTiaX2qAlj7k9eNr/pM4=
121121
github.com/authzed/cel-go v0.17.5 h1:lfpkNrR99B5QRHg5qdG9oLu/kguVlZC68VJuMk8tH9Y=
122122
github.com/authzed/cel-go v0.17.5/go.mod h1:XL/zEq5hKGVF8aOdMbG7w+BQPihLjY2W8N+UIygDA2I=
123123
github.com/authzed/consistent v0.1.0 h1:tlh1wvKoRbjRhMm2P+X5WQQyR54SRoS4MyjLOg17Mp8=

internal/services/integrationtesting/consistency_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,7 @@ func validateLookupResources(t *testing.T, vctx validationContext) {
385385
requireSameSets(t, maps.Keys(accessibleResources), maps.Keys(resolvedResources))
386386

387387
// Ensure that every returned concrete object Checks directly.
388-
bulkCheckItems := make([]*v1.BulkCheckPermissionRequestItem, 0, len(resolvedResources))
388+
checkBulkItems := make([]*v1.CheckBulkPermissionsRequestItem, 0, len(resolvedResources))
389389
expectedBulkPermissions := map[string]v1.CheckPermissionResponse_Permissionship{}
390390

391391
for _, resolvedResource := range resolvedResources {
@@ -420,7 +420,7 @@ func validateLookupResources(t *testing.T, vctx validationContext) {
420420
permissionship,
421421
)
422422

423-
bulkCheckItems = append(bulkCheckItems, &v1.BulkCheckPermissionRequestItem{
423+
checkBulkItems = append(checkBulkItems, &v1.CheckBulkPermissionsRequestItem{
424424
Resource: &v1.ObjectReference{
425425
ObjectType: resourceRelation.Namespace,
426426
ObjectId: resolvedResource.ResourceObjectId,
@@ -437,8 +437,8 @@ func validateLookupResources(t *testing.T, vctx validationContext) {
437437
}
438438

439439
// Ensure they are all found via bulk check as well.
440-
results, err := vctx.serviceTester.BulkCheck(context.Background(),
441-
bulkCheckItems,
440+
results, err := vctx.serviceTester.CheckBulk(context.Background(),
441+
checkBulkItems,
442442
vctx.revision,
443443
)
444444
require.NoError(t, err)

internal/services/integrationtesting/consistencytestutil/servicetester.go

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ type ServiceTester interface {
3030
Read(ctx context.Context, namespaceName string, atRevision datastore.Revision) ([]*core.RelationTuple, error)
3131
LookupResources(ctx context.Context, resourceRelation *core.RelationReference, subject *core.ObjectAndRelation, atRevision datastore.Revision, cursor *v1.Cursor, limit uint32) ([]*v1.LookupResourcesResponse, *v1.Cursor, error)
3232
LookupSubjects(ctx context.Context, resource *core.ObjectAndRelation, subjectRelation *core.RelationReference, atRevision datastore.Revision, caveatContext map[string]any) (map[string]*v1.LookupSubjectsResponse, error)
33+
// NOTE: ExperimentalService/BulkCheckPermission has been promoted to PermissionsService/CheckBulkPermissions
3334
BulkCheck(ctx context.Context, items []*v1.BulkCheckPermissionRequestItem, atRevision datastore.Revision) ([]*v1.BulkCheckPermissionPair, error)
35+
CheckBulk(ctx context.Context, items []*v1.CheckBulkPermissionsRequestItem, atRevision datastore.Revision) ([]*v1.CheckBulkPermissionsPair, error)
3436
}
3537

3638
func optionalizeRelation(relation string) string {
@@ -252,3 +254,19 @@ func (v1st v1ServiceTester) BulkCheck(ctx context.Context, items []*v1.BulkCheck
252254

253255
return result.Pairs, nil
254256
}
257+
258+
func (v1st v1ServiceTester) CheckBulk(ctx context.Context, items []*v1.CheckBulkPermissionsRequestItem, atRevision datastore.Revision) ([]*v1.CheckBulkPermissionsPair, error) {
259+
result, err := v1st.permClient.CheckBulkPermissions(ctx, &v1.CheckBulkPermissionsRequest{
260+
Items: items,
261+
Consistency: &v1.Consistency{
262+
Requirement: &v1.Consistency_AtLeastAsFresh{
263+
AtLeastAsFresh: zedtoken.MustNewFromRevision(atRevision),
264+
},
265+
},
266+
})
267+
if err != nil {
268+
return nil, err
269+
}
270+
271+
return result.Pairs, nil
272+
}

internal/services/v1/bulkcheck.go

Lines changed: 251 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,251 @@
1+
package v1
2+
3+
import (
4+
"context"
5+
"sync"
6+
7+
v1 "github.com/authzed/authzed-go/proto/authzed/api/v1"
8+
"github.com/jzelinskie/stringz"
9+
"google.golang.org/grpc/status"
10+
11+
"github.com/authzed/spicedb/internal/dispatch"
12+
"github.com/authzed/spicedb/internal/graph"
13+
"github.com/authzed/spicedb/internal/graph/computed"
14+
datastoremw "github.com/authzed/spicedb/internal/middleware/datastore"
15+
"github.com/authzed/spicedb/internal/middleware/usagemetrics"
16+
"github.com/authzed/spicedb/internal/namespace"
17+
"github.com/authzed/spicedb/internal/services/shared"
18+
"github.com/authzed/spicedb/internal/taskrunner"
19+
"github.com/authzed/spicedb/pkg/genutil"
20+
"github.com/authzed/spicedb/pkg/genutil/mapz"
21+
"github.com/authzed/spicedb/pkg/genutil/slicez"
22+
"github.com/authzed/spicedb/pkg/middleware/consistency"
23+
dispatchv1 "github.com/authzed/spicedb/pkg/proto/dispatch/v1"
24+
"github.com/authzed/spicedb/pkg/spiceerrors"
25+
)
26+
27+
// bulkChecker contains the logic to allow ExperimentalService/BulkCheckPermission and
28+
// PermissionsService/CheckBulkPermissions to share the same implementation.
29+
type bulkChecker struct {
30+
maxAPIDepth uint32
31+
maxCaveatContextSize int
32+
maxConcurrency uint16
33+
34+
dispatch dispatch.Dispatcher
35+
}
36+
37+
func (bc *bulkChecker) checkBulkPermissions(ctx context.Context, req *v1.CheckBulkPermissionsRequest) (*v1.CheckBulkPermissionsResponse, error) {
38+
atRevision, checkedAt, err := consistency.RevisionFromContext(ctx)
39+
if err != nil {
40+
return nil, err
41+
}
42+
43+
if len(req.Items) > maxBulkCheckCount {
44+
return nil, NewExceedsMaximumChecksErr(uint64(len(req.Items)), maxBulkCheckCount)
45+
}
46+
47+
// Compute a hash for each requested item and record its index(es) for the items, to be used for sorting of results.
48+
itemCount, err := genutil.EnsureUInt32(len(req.Items))
49+
if err != nil {
50+
return nil, err
51+
}
52+
53+
itemIndexByHash := mapz.NewMultiMapWithCap[string, int](itemCount)
54+
for index, item := range req.Items {
55+
itemHash, err := computeCheckBulkPermissionsItemHash(item)
56+
if err != nil {
57+
return nil, err
58+
}
59+
60+
itemIndexByHash.Add(itemHash, index)
61+
}
62+
63+
// Identify checks with same permission+subject over different resources and group them. This is doable because
64+
// the dispatching system already internally supports this kind of batching for performance.
65+
groupedItems, err := groupItems(ctx, groupingParameters{
66+
atRevision: atRevision,
67+
maxCaveatContextSize: bc.maxCaveatContextSize,
68+
maximumAPIDepth: bc.maxAPIDepth,
69+
}, req.Items)
70+
if err != nil {
71+
return nil, err
72+
}
73+
74+
bulkResponseMutex := sync.Mutex{}
75+
76+
tr := taskrunner.NewPreloadedTaskRunner(ctx, bc.maxConcurrency, len(groupedItems))
77+
78+
respMetadata := &dispatchv1.ResponseMeta{
79+
DispatchCount: 1,
80+
CachedDispatchCount: 0,
81+
DepthRequired: 1,
82+
DebugInfo: nil,
83+
}
84+
usagemetrics.SetInContext(ctx, respMetadata)
85+
86+
orderedPairs := make([]*v1.CheckBulkPermissionsPair, len(req.Items))
87+
88+
addPair := func(pair *v1.CheckBulkPermissionsPair) error {
89+
pairItemHash, err := computeCheckBulkPermissionsItemHash(pair.Request)
90+
if err != nil {
91+
return err
92+
}
93+
94+
found, ok := itemIndexByHash.Get(pairItemHash)
95+
if !ok {
96+
return spiceerrors.MustBugf("missing expected item hash")
97+
}
98+
99+
for _, index := range found {
100+
orderedPairs[index] = pair
101+
}
102+
103+
return nil
104+
}
105+
106+
appendResultsForError := func(params *computed.CheckParameters, resourceIDs []string, err error) error {
107+
rewritten := shared.RewriteError(ctx, err, &shared.ConfigForErrors{
108+
MaximumAPIDepth: bc.maxAPIDepth,
109+
})
110+
statusResp, ok := status.FromError(rewritten)
111+
if !ok {
112+
// If error is not a gRPC Status, fail the entire bulk check request.
113+
return err
114+
}
115+
116+
bulkResponseMutex.Lock()
117+
defer bulkResponseMutex.Unlock()
118+
119+
for _, resourceID := range resourceIDs {
120+
reqItem, err := requestItemFromResourceAndParameters(params, resourceID)
121+
if err != nil {
122+
return err
123+
}
124+
125+
if err := addPair(&v1.CheckBulkPermissionsPair{
126+
Request: reqItem,
127+
Response: &v1.CheckBulkPermissionsPair_Error{
128+
Error: statusResp.Proto(),
129+
},
130+
}); err != nil {
131+
return err
132+
}
133+
}
134+
135+
return nil
136+
}
137+
138+
appendResultsForCheck := func(params *computed.CheckParameters, resourceIDs []string, metadata *dispatchv1.ResponseMeta, results map[string]*dispatchv1.ResourceCheckResult) error {
139+
bulkResponseMutex.Lock()
140+
defer bulkResponseMutex.Unlock()
141+
142+
for _, resourceID := range resourceIDs {
143+
reqItem, err := requestItemFromResourceAndParameters(params, resourceID)
144+
if err != nil {
145+
return err
146+
}
147+
148+
if err := addPair(&v1.CheckBulkPermissionsPair{
149+
Request: reqItem,
150+
Response: pairItemFromCheckResult(results[resourceID]),
151+
}); err != nil {
152+
return err
153+
}
154+
}
155+
156+
respMetadata.DispatchCount += metadata.DispatchCount
157+
respMetadata.CachedDispatchCount += metadata.CachedDispatchCount
158+
return nil
159+
}
160+
161+
for _, group := range groupedItems {
162+
group := group
163+
164+
slicez.ForEachChunk(group.resourceIDs, MaxBulkCheckDispatchChunkSize, func(resourceIDs []string) {
165+
tr.Add(func(ctx context.Context) error {
166+
ds := datastoremw.MustFromContext(ctx).SnapshotReader(atRevision)
167+
168+
// Ensure the check namespaces and relations are valid.
169+
err := namespace.CheckNamespaceAndRelations(ctx,
170+
[]namespace.TypeAndRelationToCheck{
171+
{
172+
NamespaceName: group.params.ResourceType.Namespace,
173+
RelationName: group.params.ResourceType.Relation,
174+
AllowEllipsis: false,
175+
},
176+
{
177+
NamespaceName: group.params.Subject.Namespace,
178+
RelationName: stringz.DefaultEmpty(group.params.Subject.Relation, graph.Ellipsis),
179+
AllowEllipsis: true,
180+
},
181+
}, ds)
182+
if err != nil {
183+
return appendResultsForError(group.params, resourceIDs, err)
184+
}
185+
186+
// Call bulk check to compute the check result(s) for the resource ID(s).
187+
rcr, metadata, err := computed.ComputeBulkCheck(ctx, bc.dispatch, *group.params, resourceIDs)
188+
if err != nil {
189+
return appendResultsForError(group.params, resourceIDs, err)
190+
}
191+
192+
return appendResultsForCheck(group.params, resourceIDs, metadata, rcr)
193+
})
194+
})
195+
}
196+
197+
// Run the checks in parallel.
198+
if err := tr.StartAndWait(); err != nil {
199+
return nil, err
200+
}
201+
202+
return &v1.CheckBulkPermissionsResponse{CheckedAt: checkedAt, Pairs: orderedPairs}, nil
203+
}
204+
205+
func toCheckBulkPermissionsRequest(req *v1.BulkCheckPermissionRequest) *v1.CheckBulkPermissionsRequest {
206+
items := make([]*v1.CheckBulkPermissionsRequestItem, len(req.Items))
207+
for i, item := range req.Items {
208+
items[i] = &v1.CheckBulkPermissionsRequestItem{
209+
Resource: item.Resource,
210+
Permission: item.Permission,
211+
Subject: item.Subject,
212+
Context: item.Context,
213+
}
214+
}
215+
216+
return &v1.CheckBulkPermissionsRequest{Items: items}
217+
}
218+
219+
func toBulkCheckPermissionResponse(resp *v1.CheckBulkPermissionsResponse) *v1.BulkCheckPermissionResponse {
220+
pairs := make([]*v1.BulkCheckPermissionPair, len(resp.Pairs))
221+
for i, pair := range resp.Pairs {
222+
pairs[i] = &v1.BulkCheckPermissionPair{}
223+
pairs[i].Request = &v1.BulkCheckPermissionRequestItem{
224+
Resource: pair.Request.Resource,
225+
Permission: pair.Request.Permission,
226+
Subject: pair.Request.Subject,
227+
Context: pair.Request.Context,
228+
}
229+
230+
switch t := pair.Response.(type) {
231+
case *v1.CheckBulkPermissionsPair_Item:
232+
pairs[i].Response = &v1.BulkCheckPermissionPair_Item{
233+
Item: &v1.BulkCheckPermissionResponseItem{
234+
Permissionship: t.Item.Permissionship,
235+
PartialCaveatInfo: t.Item.PartialCaveatInfo,
236+
},
237+
}
238+
case *v1.CheckBulkPermissionsPair_Error:
239+
pairs[i].Response = &v1.BulkCheckPermissionPair_Error{
240+
Error: t.Error,
241+
}
242+
default:
243+
panic("unknown CheckBulkPermissionResponse pair response type")
244+
}
245+
}
246+
247+
return &v1.BulkCheckPermissionResponse{
248+
CheckedAt: resp.CheckedAt,
249+
Pairs: pairs,
250+
}
251+
}

0 commit comments

Comments
 (0)