Skip to content

Commit

Permalink
Handle functioned arrows in warnings system
Browse files Browse the repository at this point in the history
  • Loading branch information
@josephschorr
josephschorr committed Jul 31, 2024
1 parent 1779380 commit a51362f
Show file tree
Hide file tree
Showing 3 changed files with 133 additions and 28 deletions.
64 changes: 38 additions & 26 deletions pkg/development/warningdefs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ var lintPermissionReferencingItself = computedUsersetCheck{
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)
permName := parentRelation.Name
if computedUserset.Relation == permName {
if computedUserset.GetRelation() == permName {
return warningForPosition(
"permission-references-itself",
fmt.Sprintf("Permission %q references itself, which will cause an error to be raised due to infinite recursion", permName),
Expand All @@ -68,13 +68,13 @@ var lintArrowReferencingUnreachable = ttuCheck{
"arrow-references-unreachable-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
ttu ttu,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
referencedRelation, ok := ts.GetRelation(ttu.GetTupleset().GetRelation())
if !ok {
return nil, nil
}
Expand All @@ -91,24 +91,28 @@ var lintArrowReferencingUnreachable = ttuCheck{
return nil, err
}

_, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
_, ok := nts.GetRelation(ttu.GetComputedUserset().GetRelation())
if ok {
wasFound = true
}
}

if !wasFound {
arrowString, err := ttu.GetArrowString()
if err != nil {
return nil, err
}

return warningForPosition(
"arrow-references-unreachable-relation",
fmt.Sprintf(
"Arrow `%s->%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
"Arrow `%s` under permission %q references relation/permission %q that does not exist on any subject types of relation %q",
arrowString,
parentRelation.Name,
ttu.ComputedUserset.Relation,
ttu.Tupleset.Relation,
ttu.GetComputedUserset().GetRelation(),
ttu.GetTupleset().GetRelation(),
),
fmt.Sprintf("%s->%s", ttu.Tupleset.Relation, ttu.ComputedUserset.Relation),
arrowString,
sourcePosition,
), nil
}
Expand All @@ -121,13 +125,13 @@ var lintArrowOverSubRelation = ttuCheck{
"arrow-walks-subject-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
ttu ttu,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
referencedRelation, ok := ts.GetRelation(ttu.GetTupleset().GetRelation())
if !ok {
return nil, nil
}
Expand All @@ -137,20 +141,24 @@ var lintArrowOverSubRelation = ttuCheck{
return nil, err
}

arrowString, err := ttu.GetArrowString()
if err != nil {
return nil, err
}

for _, subjectType := range allowedSubjectTypes {
if subjectType.Relation != tuple.Ellipsis {
if subjectType.GetRelation() != tuple.Ellipsis {
return warningForPosition(
"arrow-walks-subject-relation",
fmt.Sprintf(
"Arrow `%s->%s` under permission %q references relation %q that has relation %q on subject %q: *the subject relation will be ignored for the arrow*",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
"Arrow `%s` under permission %q references relation %q that has relation %q on subject %q: *the subject relation will be ignored for the arrow*",
arrowString,
parentRelation.Name,
ttu.Tupleset.Relation,
subjectType.Relation,
ttu.GetTupleset().GetRelation(),
subjectType.GetRelation(),
subjectType.Namespace,
),
fmt.Sprintf("%s->%s", ttu.Tupleset.Relation, ttu.ComputedUserset.Relation),
arrowString,
sourcePosition,
), nil
}
Expand All @@ -164,13 +172,13 @@ var lintArrowReferencingRelation = ttuCheck{
"arrow-references-relation",
func(
ctx context.Context,
ttu *corev1.TupleToUserset,
ttu ttu,
sourcePosition *corev1.SourcePosition,
ts *typesystem.TypeSystem,
) (*devinterface.DeveloperWarning, error) {
parentRelation := ctx.Value(relationKey).(*corev1.Relation)

referencedRelation, ok := ts.GetRelation(ttu.Tupleset.Relation)
referencedRelation, ok := ts.GetRelation(ttu.GetTupleset().GetRelation())
if !ok {
return nil, nil
}
Expand All @@ -182,13 +190,18 @@ var lintArrowReferencingRelation = ttuCheck{
return nil, err
}

arrowString, err := ttu.GetArrowString()
if err != nil {
return nil, err
}

for _, subjectType := range allowedSubjectTypes {
nts, err := ts.TypeSystemForNamespace(ctx, subjectType.Namespace)
if err != nil {
return nil, err
}

targetRelation, ok := nts.GetRelation(ttu.ComputedUserset.Relation)
targetRelation, ok := nts.GetRelation(ttu.GetComputedUserset().GetRelation())
if !ok {
continue
}
Expand All @@ -197,14 +210,13 @@ var lintArrowReferencingRelation = ttuCheck{
return warningForPosition(
"arrow-references-relation",
fmt.Sprintf(
"Arrow `%s->%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
ttu.Tupleset.Relation,
ttu.ComputedUserset.Relation,
"Arrow `%s` under permission %q references relation %q on definition %q; it is recommended to point to a permission",
arrowString,
parentRelation.Name,
targetRelation.Name,
subjectType.Namespace,
),
fmt.Sprintf("%s->%s", ttu.Tupleset.Relation, ttu.ComputedUserset.Relation),
arrowString,
sourcePosition,
), nil
}
Expand Down
76 changes: 74 additions & 2 deletions pkg/development/warnings.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package development

import (
"context"
"fmt"

"github.com/authzed/spicedb/pkg/namespace"
corev1 "github.com/authzed/spicedb/pkg/proto/core/v1"
Expand Down Expand Up @@ -120,10 +121,20 @@ func shouldSkipCheck(metadata *corev1.Metadata, name string) bool {
return false
}

type tupleset interface {
GetRelation() string
}

type ttu interface {
GetTupleset() tupleset
GetComputedUserset() *corev1.ComputedUserset
GetArrowString() (string, error)
}

type (
relationChecker func(ctx context.Context, relation *corev1.Relation, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
computedUsersetChecker func(ctx context.Context, computedUserset *corev1.ComputedUserset, sourcePosition *corev1.SourcePosition, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
ttuChecker func(ctx context.Context, ttu *corev1.TupleToUserset, sourcePosition *corev1.SourcePosition, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
ttuChecker func(ctx context.Context, ttu ttu, sourcePosition *corev1.SourcePosition, vts *typesystem.TypeSystem) (*devinterface.DeveloperWarning, error)
)

type relationCheck struct {
Expand Down Expand Up @@ -198,13 +209,29 @@ func walkUsersetOperations(ctx context.Context, ops []*corev1.SetOperation_Child

warnings = append(warnings, found...)

case *corev1.SetOperation_Child_FunctionedTupleToUserset:
for _, check := range checks.ttuChecks {
if shouldSkipCheck(relation.Metadata, check.name) {
continue
}

checkerWarning, err := check.fn(ctx, wrappedFunctionedTTU{t.FunctionedTupleToUserset}, op.SourcePosition, ts)
if err != nil {
return nil, err
}

if checkerWarning != nil {
warnings = append(warnings, checkerWarning)
}
}

case *corev1.SetOperation_Child_TupleToUserset:
for _, check := range checks.ttuChecks {
if shouldSkipCheck(relation.Metadata, check.name) {
continue
}

checkerWarning, err := check.fn(ctx, t.TupleToUserset, op.SourcePosition, ts)
checkerWarning, err := check.fn(ctx, wrappedTTU{t.TupleToUserset}, op.SourcePosition, ts)
if err != nil {
return nil, err
}
Expand All @@ -224,3 +251,48 @@ func walkUsersetOperations(ctx context.Context, ops []*corev1.SetOperation_Child

return warnings, nil
}

type wrappedFunctionedTTU struct {
*corev1.FunctionedTupleToUserset
}

func (wfttu wrappedFunctionedTTU) GetTupleset() tupleset {
return wfttu.FunctionedTupleToUserset.GetTupleset()
}

func (wfttu wrappedFunctionedTTU) GetComputedUserset() *corev1.ComputedUserset {
return wfttu.FunctionedTupleToUserset.GetComputedUserset()
}

func (wfttu wrappedFunctionedTTU) GetArrowString() (string, error) {
var functionName string
switch wfttu.Function {
case corev1.FunctionedTupleToUserset_FUNCTION_ANY:
functionName = "any"

case corev1.FunctionedTupleToUserset_FUNCTION_ALL:
functionName = "all"

default:
return "", spiceerrors.MustBugf("unknown function type %T", wfttu.Function)
}

return fmt.Sprintf("%s.%s(%s)", wfttu.GetTupleset().GetRelation(), functionName, wfttu.GetComputedUserset().GetRelation()), nil
}

type wrappedTTU struct {
*corev1.TupleToUserset
}

func (wtu wrappedTTU) GetTupleset() tupleset {
return wtu.TupleToUserset.GetTupleset()
}

func (wtu wrappedTTU) GetComputedUserset() *corev1.ComputedUserset {
return wtu.TupleToUserset.GetComputedUserset()
}

func (wtu wrappedTTU) GetArrowString() (string, error) {
arrowString := fmt.Sprintf("%s->%s", wtu.GetTupleset().GetRelation(), wtu.GetComputedUserset().GetRelation())
return arrowString, nil
}
21 changes: 21 additions & 0 deletions pkg/development/warnings_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,27 @@ func TestWarnings(t *testing.T) {
SourceCode: "parent_group->member",
},
},
{
name: "all arrow referencing subject relation",
schema: `definition group {
relation direct_member: user
permission member = direct_member
}
definition user {}
definition document {
relation parent_group: group#member
permission view = parent_group.all(member)
}
`,
expectedWarning: &developerv1.DeveloperWarning{
Message: "Arrow `parent_group.all(member)` under permission \"view\" references relation \"parent_group\" that has relation \"member\" on subject \"group\": *the subject relation will be ignored for the arrow* (arrow-walks-subject-relation)",
Line: 10,
Column: 23,
SourceCode: "parent_group.all(member)",
},
},
{
name: "relation referencing its parent definition in its name",
schema: `definition user {}
Expand Down

0 comments on commit a51362f

Please sign in to comment.