Skip to content

Commit

Permalink
introduce caveat support in WriteRelationships
Browse files Browse the repository at this point in the history
this commit changes parts of the datastore implementation.
The duality caveatID/CaveatName was problematic to handle
between the controller layer and the datastore layer:

The usage of ID introduced a situation where a core.RelationTuple
cannot be converted to a v1.Relationship without a database lookup,
because database tuples reference caveats via ID, but we return
named caveats to the client via v1.Relationship. It would be a departure
from what happens today in service controllers: v1-to-core would introduce
I/O operations.

The question here is that it's not clear the usage of ID buys us anything
at this point. The rationale behind them was being able to support "anonymous caveats".
An anonymous caveat signature defined in place in a relation in the schema.
Because they don't have a name, we need a way to identify them.

However, it's not clear anonymous caveats is needed at this point,
and that we should work on it when the need arises.
  • Loading branch information
vroldanbet committed Sep 29, 2022
1 parent d11f5ca commit 679697f
Show file tree
Hide file tree
Showing 17 changed files with 585 additions and 532 deletions.
2 changes: 1 addition & 1 deletion e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/authzed/spicedb/e2e
go 1.18

require (
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5
github.com/authzed/spicedb v1.5.0
github.com/brianvoe/gofakeit/v6 v6.15.0
Expand Down
4 changes: 2 additions & 2 deletions e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ github.com/BurntSushi/xgb v0.0.0-20160522181843-27f122750802/go.mod h1:IVnqGOEym
github.com/Masterminds/semver/v3 v3.1.1/go.mod h1:VPu/7SZ7ePZ3QOrcuXROw5FAcLl4a0cBrbBpGY/8hQs=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/antihax/optional v1.0.0/go.mod h1:uupD/76wgC+ih3iEmQUL+0Ugr19nfwCT1kdvxnR2qWY=
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25 h1:sPxDxgGGf+67cXtPCZYypJ1mFdYxqC5Vn8PDt5yzo88=
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58 h1:2LtqgHybN/xrEBVpI0y+HzMzEe4mBYODK1D0FVCj+GM=
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5 h1:sZM7XzdyuLyxj7pC/g7uX+XAqZ7m6NMxZzuQRovgBPw=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE=
github.com/brianvoe/gofakeit/v6 v6.15.0 h1:lJPGJZ2/07TRGDazyTzD5b18N3y4tmmJpdhCUw18FlI=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ require (
cloud.google.com/go/spanner v1.37.0
github.com/IBM/pgxpoolprometheus v1.0.1
github.com/Masterminds/squirrel v1.5.3
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5
github.com/aws/aws-sdk-go v1.44.90
github.com/benbjohnson/clock v1.3.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,8 @@ github.com/antlr/antlr4/runtime/Go/antlr v0.0.0-20220418222510-f25a4f6275ed h1:u
github.com/antlr/antlr4/runtime/Go/antlr v0.0.0-20220418222510-f25a4f6275ed/go.mod h1:F7bn7fEU90QkQ3tnmaTx3LTKLEDqnwWODIYppRQ5hnY=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25 h1:sPxDxgGGf+67cXtPCZYypJ1mFdYxqC5Vn8PDt5yzo88=
github.com/authzed/authzed-go v0.7.1-0.20220915201801-94eebb436d25/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58 h1:2LtqgHybN/xrEBVpI0y+HzMzEe4mBYODK1D0FVCj+GM=
github.com/authzed/authzed-go v0.7.1-0.20220928193058-a55432766d58/go.mod h1:h9Zar1MSSrVsqbcbE5/RO7gpg6Fx5QYW2C5QduSox5M=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5 h1:sZM7XzdyuLyxj7pC/g7uX+XAqZ7m6NMxZzuQRovgBPw=
github.com/authzed/grpcutil v0.0.0-20220104222419-f813f77722e5/go.mod h1:rqjY3zyK/YP7NID9+B2BdIRRkvnK+cdf9/qya/zaFZE=
github.com/aws/aws-sdk-go v1.17.4/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
Expand Down
55 changes: 7 additions & 48 deletions internal/datastore/memdb/caveat.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
package memdb

import (
"errors"
"time"

"github.com/authzed/spicedb/pkg/datastore"
core "github.com/authzed/spicedb/pkg/proto/core/v1"

Expand All @@ -13,7 +10,6 @@ import (
const tableCaveats = "caveats"

type caveat struct {
id datastore.CaveatID
name string
expression []byte
}
Expand All @@ -36,31 +32,8 @@ func (r *memdbReader) ReadCaveatByName(name string) (*core.Caveat, error) {
return r.readUnwrappedCaveatByName(tx, name)
}

func (r *memdbReader) ReadCaveatByID(ID datastore.CaveatID) (*core.Caveat, error) {
r.lockOrPanic()
defer r.Unlock()

tx, err := r.txSource()
if err != nil {
return nil, err
}
return r.readCaveatByID(tx, ID)
}

func (r *memdbReader) readCaveatByID(tx *memdb.Txn, ID datastore.CaveatID) (*core.Caveat, error) {
found, err := tx.First(tableCaveats, indexID, ID)
if err != nil {
return nil, err
}
if found == nil {
return nil, datastore.NewCaveatIDNotFoundErr(ID)
}
c := found.(*caveat)
return c.Unwrap(), nil
}

func (r *memdbReader) readCaveatByName(tx *memdb.Txn, name string) (*caveat, error) {
found, err := tx.First(tableCaveats, indexName, name)
found, err := tx.First(tableCaveats, indexID, name)
if err != nil {
return nil, err
}
Expand All @@ -78,41 +51,27 @@ func (r *memdbReader) readUnwrappedCaveatByName(tx *memdb.Txn, name string) (*co
return c.Unwrap(), nil
}

func (rwt *memdbReadWriteTx) WriteCaveats(caveats []*core.Caveat) ([]datastore.CaveatID, error) {
func (rwt *memdbReadWriteTx) WriteCaveats(caveats []*core.Caveat) error {
rwt.lockOrPanic()
defer rwt.Unlock()
tx, err := rwt.txSource()
if err != nil {
return nil, err
return err
}
return rwt.writeCaveat(tx, caveats)
}

func (rwt *memdbReadWriteTx) writeCaveat(tx *memdb.Txn, caveats []*core.Caveat) ([]datastore.CaveatID, error) {
ids := make([]datastore.CaveatID, 0, len(caveats))
func (rwt *memdbReadWriteTx) writeCaveat(tx *memdb.Txn, caveats []*core.Caveat) error {
for _, coreCaveat := range caveats {
id := datastore.CaveatID(time.Now().UnixNano())
c := caveat{
id: id,
name: coreCaveat.Name,
expression: coreCaveat.Expression,
}
// in order to implement upserts we need to determine the ID of the previously
// stored caveat
found, err := rwt.readCaveatByName(tx, coreCaveat.Name)
if err != nil && !errors.As(err, &datastore.ErrCaveatNameNotFound{}) {
return nil, err
}
if found != nil {
id = found.id
c.id = id
}
if err = tx.Insert(tableCaveats, &c); err != nil {
return nil, err
if err := tx.Insert(tableCaveats, &c); err != nil {
return err
}
ids = append(ids, id)
}
return ids, nil
return nil
}

func (rwt *memdbReadWriteTx) DeleteCaveats(caveats []*core.Caveat) error {
Expand Down
67 changes: 28 additions & 39 deletions internal/datastore/memdb/caveat_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package memdb

import (
"context"
"math"
"testing"
"time"

"github.com/google/go-cmp/cmp"
"google.golang.org/protobuf/testing/protocmp"

"github.com/authzed/spicedb/internal/datastore/common"
"github.com/authzed/spicedb/internal/testfixtures"
"github.com/authzed/spicedb/pkg/caveats"
Expand All @@ -27,16 +29,11 @@ func TestWriteReadCaveat(t *testing.T) {
// Dupes in same transaction are treated as upserts
coreCaveat := createCoreCaveat(t)
ctx := context.Background()
_, ids, err := writeCaveats(ctx, ds, coreCaveat, coreCaveat)
req.NoError(err)
req.Equal(ids[0], ids[1]) // dupe caveats generate same IDs

// Succeeds writing a caveat
rev, ID, err := writeCaveat(ctx, ds, coreCaveat)
_, err = writeCaveats(ctx, ds, coreCaveat, coreCaveat)
req.NoError(err)

// Writing same named caveat in different tx is treated as upsert
_, _, err = writeCaveat(ctx, ds, coreCaveat)
// Succeeds upserting an existing caveat
rev, err := writeCaveat(ctx, ds, coreCaveat)
req.NoError(err)

// The caveat can be looked up by name
Expand All @@ -45,18 +42,12 @@ func TestWriteReadCaveat(t *testing.T) {

cv, err := cr.ReadCaveatByName(coreCaveat.Name)
req.NoError(err)
req.Equal(coreCaveat, cv)

// The caveat can be looked up by ID
cv, err = cr.ReadCaveatByID(ID)
req.NoError(err)
req.Equal(coreCaveat, cv)
foundDiff := cmp.Diff(coreCaveat, cv, protocmp.Transform())
req.Empty(foundDiff)

// Returns an error if caveat name or ID does not exist
_, err = cr.ReadCaveatByName("doesnotexist")
req.ErrorAs(err, &datastore.ErrCaveatNameNotFound{})
_, err = cr.ReadCaveatByID(math.MaxUint64)
req.ErrorAs(err, &datastore.ErrCaveatIDNotFound{})
}

func TestWriteCaveatedTuple(t *testing.T) {
Expand All @@ -69,10 +60,10 @@ func TestWriteCaveatedTuple(t *testing.T) {

// Store caveat, write caveated tuple and read back same value
coreCaveat := createCoreCaveat(t)
_, cavID, err := writeCaveat(ctx, ds, coreCaveat)
_, err = writeCaveat(ctx, ds, coreCaveat)
req.NoError(err)

tpl := createTestCaveatedTuple(t, "document:companyplan#parent@folder:company#...", cavID)
tpl := createTestCaveatedTuple(t, "document:companyplan#parent@folder:company#...", coreCaveat.Name)
rev, err := common.WriteTuples(ctx, sds, core.RelationTupleUpdate_CREATE, tpl)
req.NoError(err)
iter, err := ds.SnapshotReader(rev).QueryRelationships(ctx, datastore.RelationshipsFilter{
Expand All @@ -82,10 +73,11 @@ func TestWriteCaveatedTuple(t *testing.T) {

defer iter.Close()
readTpl := iter.Next()
req.Equal(tpl, readTpl)
foundDiff := cmp.Diff(tpl, readTpl, protocmp.Transform())
req.Empty(foundDiff)

// Caveated tuple can reference non-existing caveat - controller layer is responsible for validation
tpl = createTestCaveatedTuple(t, "document:rando#parent@folder:company#...", math.MaxUint64)
tpl = createTestCaveatedTuple(t, "document:rando#parent@folder:company#...", "rando")
_, err = common.WriteTuples(ctx, sds, core.RelationTupleUpdate_CREATE, tpl)
req.NoError(err)
}
Expand All @@ -99,66 +91,63 @@ func TestCaveatSnapshotReads(t *testing.T) {
// Write an initial caveat
coreCaveat := createCoreCaveat(t)
ctx := context.Background()
oldRev, oldID, err := writeCaveat(ctx, ds, coreCaveat)
oldRev, err := writeCaveat(ctx, ds, coreCaveat)
req.NoError(err)

// Modify caveat and update
oldExpression := coreCaveat.Expression
newExpression := []byte{0x0a}
coreCaveat.Expression = newExpression
newRev, newID, err := writeCaveat(ctx, ds, coreCaveat)
newRev, err := writeCaveat(ctx, ds, coreCaveat)
req.NoError(err)

// check most recent revision
cr, ok := ds.SnapshotReader(newRev).(datastore.CaveatReader)
req.True(ok, "expected a CaveatStorer value")
cv, err := cr.ReadCaveatByID(oldID)
cv, err := cr.ReadCaveatByName(coreCaveat.Name)
req.NoError(err)
req.Equal(newExpression, cv.Expression)

// check previous revision
cr, ok = ds.SnapshotReader(oldRev).(datastore.CaveatReader)
req.True(ok, "expected a CaveatStorer value")
cv, err = cr.ReadCaveatByID(newID)
cv, err = cr.ReadCaveatByName(coreCaveat.Name)
req.NoError(err)
req.Equal(oldExpression, cv.Expression)
}

func createTestCaveatedTuple(t *testing.T, tplString string, id datastore.CaveatID) *core.RelationTuple {
func createTestCaveatedTuple(t *testing.T, tplString string, caveatName string) *core.RelationTuple {
tpl := tuple.MustParse(tplString)
st, err := structpb.NewStruct(map[string]interface{}{"a": 1, "b": "test"})
require.NoError(t, err)

tpl.Caveat = &core.ContextualizedCaveat{
CaveatId: uint64(id),
Context: st,
CaveatName: caveatName,
Context: st,
}
return tpl
}

func writeCaveats(ctx context.Context, ds datastore.Datastore, coreCaveat ...*core.Caveat) (datastore.Revision, []datastore.CaveatID, error) {
var IDs []datastore.CaveatID
func writeCaveats(ctx context.Context, ds datastore.Datastore, coreCaveat ...*core.Caveat) (datastore.Revision, error) {
rev, err := ds.ReadWriteTx(ctx, func(ctx context.Context, tx datastore.ReadWriteTransaction) error {
cs, ok := tx.(datastore.CaveatStorer)
if !ok {
panic("expected a CaveatStorer value")
}
var err error
IDs, err = cs.WriteCaveats(coreCaveat)
return err
return cs.WriteCaveats(coreCaveat)
})
if err != nil {
return datastore.NoRevision, nil, err
return datastore.NoRevision, err
}
return rev, IDs, err
return rev, err
}

func writeCaveat(ctx context.Context, ds datastore.Datastore, coreCaveat *core.Caveat) (datastore.Revision, datastore.CaveatID, error) {
rev, ids, err := writeCaveats(ctx, ds, coreCaveat)
func writeCaveat(ctx context.Context, ds datastore.Datastore, coreCaveat *core.Caveat) (datastore.Revision, error) {
rev, err := writeCaveats(ctx, ds, coreCaveat)
if err != nil {
return datastore.NoRevision, 0, err
return datastore.NoRevision, err
}
return rev, ids[0], nil
return rev, nil
}

func createCoreCaveat(t *testing.T) *core.Caveat {
Expand Down
4 changes: 2 additions & 2 deletions internal/datastore/memdb/readwrite.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,8 @@ func (rwt *memdbReadWriteTx) toCaveatReference(mutation *core.RelationTupleUpdat
var cr *contextualizedCaveat
if mutation.Tuple.Caveat != nil {
cr = &contextualizedCaveat{
caveatID: datastore.CaveatID(mutation.Tuple.Caveat.CaveatId),
context: mutation.Tuple.Caveat.Context.AsMap(),
caveatName: mutation.Tuple.Caveat.CaveatName,
context: mutation.Tuple.Caveat.Context.AsMap(),
}
}
return cr
Expand Down
14 changes: 4 additions & 10 deletions internal/datastore/memdb/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

const (
tableNamespace = "namespace"
indexName = "name"

tableRelationship = "relationship"
indexID = "id"
Expand Down Expand Up @@ -50,8 +49,8 @@ type relationship struct {
}

type contextualizedCaveat struct {
caveatID datastore.CaveatID
context map[string]any
caveatName string
context map[string]any
}

func (cr *contextualizedCaveat) ContextualizedCaveat() (*core.ContextualizedCaveat, error) {
Expand All @@ -63,8 +62,8 @@ func (cr *contextualizedCaveat) ContextualizedCaveat() (*core.ContextualizedCave
return nil, err
}
return &core.ContextualizedCaveat{
CaveatId: uint64(cr.caveatID),
Context: v,
CaveatName: cr.caveatName,
Context: v,
}, nil
}

Expand Down Expand Up @@ -210,11 +209,6 @@ var schema = &memdb.DBSchema{
indexID: {
Name: indexID,
Unique: true,
Indexer: &memdb.UintFieldIndex{Field: "id"},
},
indexName: {
Name: indexName,
Unique: true,
Indexer: &memdb.StringFieldIndex{Field: "name"},
},
},
Expand Down
Loading

0 comments on commit 679697f

Please sign in to comment.