Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some small reachable resources and lookup improvements #620

Merged
merged 4 commits into from
May 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions internal/dispatch/graph/lookup_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestSimpleLookup(t *testing.T) {
ONR("document", "masterplan", "viewer"),
},
2,
2,
1,
},
{
RR("document", "owner"),
Expand All @@ -66,7 +66,7 @@ func TestSimpleLookup(t *testing.T) {
ONR("document", "masterplan", "owner"),
},
2,
2,
1,
},
{
RR("document", "viewer"),
Expand All @@ -76,7 +76,7 @@ func TestSimpleLookup(t *testing.T) {
ONR("document", "masterplan", "viewer"),
},
6,
4,
3,
},
{
RR("document", "viewer_and_editor"),
Expand Down
128 changes: 71 additions & 57 deletions internal/graph/reachableresources.go
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ func (crr *ConcurrentReachableResources) ReachableResources(
for _, entrypoint := range entrypoints {
switch entrypoint.EntrypointKind() {
case core.ReachabilityEntrypoint_RELATION_ENTRYPOINT:
err := crr.lookupRelationEntrypoint(subCtx, entrypoint, g, reader, req, stream)
err := crr.lookupRelationEntrypoint(subCtx, entrypoint, rg, g, reader, req, stream)
if err != nil {
return err
}
Expand All @@ -96,43 +96,28 @@ func (crr *ConcurrentReachableResources) ReachableResources(
Relation: containingRelation.Relation,
}

// Otherwise, redispatch.
g.Go(crr.redispatch(
subCtx,
entrypoint,
stream,
&v1.DispatchReachableResourcesRequest{
ObjectRelation: req.ObjectRelation,
Subject: rewrittenSubjectTpl,
Metadata: &v1.ResolverMeta{
AtRevision: req.Revision.String(),
DepthRemaining: req.Metadata.DepthRemaining - 1,
},
},
))
err := crr.redispatchOrReport(subCtx, rewrittenSubjectTpl, rg, g, entrypoint, stream, req)
if err != nil {
return err
}

case core.ReachabilityEntrypoint_TUPLESET_TO_USERSET_ENTRYPOINT:
containingRelation := entrypoint.ContainingRelationOrPermission()

// TODO(jschorr): Should we put this information into the entrypoint itself, to avoid
// a lookup of the namespace?
nsDef, ttuTypeSystem, err := namespace.ReadNamespaceAndTypes(ctx, containingRelation.Namespace, reader)
_, ttuTypeSystem, err := namespace.ReadNamespaceAndTypes(ctx, containingRelation.Namespace, reader)
if err != nil {
return err
}

ttu := entrypoint.TupleToUserset(nsDef)
if ttu == nil {
return fmt.Errorf("found nil ttu for TTU entrypoint")
}
tuplesetRelation := entrypoint.TuplesetRelation()

// Search for the resolved subject in the tupleset of the TTU. Note that we need to do so
// for both `...` as well as the subject's defined relation, as either is applicable in
// the tupleset (the relation is ignored when following the arrow).
relations := strset.New(tuple.Ellipsis, req.Subject.Relation)

for _, subjectRelation := range relations.List() {
isAllowed, err := ttuTypeSystem.IsAllowedDirectRelation(ttu.Tupleset.Relation, req.Subject.Namespace, subjectRelation)
isAllowed, err := ttuTypeSystem.IsAllowedDirectRelation(tuplesetRelation, req.Subject.Namespace, subjectRelation)
if err != nil {
return err
}
Expand All @@ -150,7 +135,7 @@ func (crr *ConcurrentReachableResources) ReachableResources(
}),
options.WithResRelation(&options.ResourceRelation{
Namespace: containingRelation.Namespace,
Relation: ttu.Tupleset.Relation,
Relation: tuplesetRelation,
}),
)
if err != nil {
Expand All @@ -169,19 +154,10 @@ func (crr *ConcurrentReachableResources) ReachableResources(
Relation: containingRelation.Relation,
}

g.Go(crr.redispatch(
subCtx,
entrypoint,
stream,
&v1.DispatchReachableResourcesRequest{
ObjectRelation: req.ObjectRelation,
Subject: rewrittenObjectTpl,
Metadata: &v1.ResolverMeta{
AtRevision: req.Revision.String(),
DepthRemaining: req.Metadata.DepthRemaining - 1,
},
},
))
err := crr.redispatchOrReport(subCtx, rewrittenObjectTpl, rg, g, entrypoint, stream, req)
if err != nil {
return err
}
}
}

Expand All @@ -195,6 +171,7 @@ func (crr *ConcurrentReachableResources) ReachableResources(

func (crr *ConcurrentReachableResources) lookupRelationEntrypoint(ctx context.Context,
entrypoint namespace.ReachabilityEntrypoint,
rg *namespace.ReachabilityGraph,
g *errgroup.Group,
reader datastore.Reader,
req ValidatedReachableResourcesRequest,
Expand Down Expand Up @@ -241,20 +218,10 @@ func (crr *ConcurrentReachableResources) lookupRelationEntrypoint(ctx context.Co
return it.Err()
}

// Redispatch to continue looking for results.
g.Go(crr.redispatch(
ctx,
entrypoint,
stream,
&v1.DispatchReachableResourcesRequest{
ObjectRelation: req.ObjectRelation,
Subject: tpl.ObjectAndRelation,
Metadata: &v1.ResolverMeta{
AtRevision: req.Revision.String(),
DepthRemaining: req.Metadata.DepthRemaining - 1,
},
},
))
err := crr.redispatchOrReport(ctx, tpl.ObjectAndRelation, rg, g, entrypoint, stream, req)
if err != nil {
return err
}
}
return nil
}
Expand All @@ -274,13 +241,52 @@ func (crr *ConcurrentReachableResources) lookupRelationEntrypoint(ctx context.Co
return nil
}

func (crr *ConcurrentReachableResources) redispatch(
// redispatchOrReport checks if further redispatching is necessary for the found resource
// type. If not, and the found resource type+relation matches the target resource type+relation,
// the resource is reported to the parent stream.
func (crr *ConcurrentReachableResources) redispatchOrReport(
ctx context.Context,
foundResource *core.ObjectAndRelation,
rg *namespace.ReachabilityGraph,
g *errgroup.Group,
entrypoint namespace.ReachabilityEntrypoint,
parentStream dispatch.ReachableResourcesStream,
req *v1.DispatchReachableResourcesRequest,
) func() error {
return func() error {
parentRequest ValidatedReachableResourcesRequest,
) error {
// Check for entrypoints for the new found resource type.
hasResourceEntrypoints, err := rg.HasOptimizedEntrypointsForSubjectToResource(ctx, &core.RelationReference{
Namespace: foundResource.Namespace,
Relation: foundResource.Relation,
}, parentRequest.ObjectRelation)
if err != nil {
return err
}

// If there are no entrypoints, then no further dispatch is necessary.
if !hasResourceEntrypoints {
// If the found resource matches the target resource type and relation, yield the resource.
if foundResource.Namespace == parentRequest.ObjectRelation.Namespace &&
foundResource.Relation == parentRequest.ObjectRelation.Relation {
status := v1.ReachableResource_REQUIRES_CHECK
if entrypoint.IsDirectResult() {
status = v1.ReachableResource_HAS_PERMISSION
}

return parentStream.Publish(&v1.DispatchReachableResourcesResponse{
Resource: &v1.ReachableResource{
Resource: foundResource,
ResultStatus: status,
},
Metadata: emptyMetadata,
})
}

// Otherwise, we're done.
return nil
}

// Otherwise, redispatch.
g.Go(func() error {
stream := &dispatch.WrappedDispatchStream[*v1.DispatchReachableResourcesResponse]{
Stream: parentStream,
Ctx: ctx,
Expand All @@ -302,6 +308,14 @@ func (crr *ConcurrentReachableResources) redispatch(
},
}

return crr.d.DispatchReachableResources(req, stream)
}
return crr.d.DispatchReachableResources(&v1.DispatchReachableResourcesRequest{
ObjectRelation: parentRequest.ObjectRelation,
Subject: foundResource,
Metadata: &v1.ResolverMeta{
AtRevision: parentRequest.Revision.String(),
DepthRemaining: parentRequest.Metadata.DepthRemaining - 1,
},
}, stream)
})
return nil
}
52 changes: 0 additions & 52 deletions internal/namespace/oppath.go

This file was deleted.

Loading