Skip to content

Commit

Permalink
Add API support for transaction metadata on WriteRels and DeleteRels
Browse files Browse the repository at this point in the history
  • Loading branch information
josephschorr committed Oct 4, 2024
1 parent 9d87aa6 commit bb6362c
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 10 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.22.7

require (
github.com/authzed/authzed-go v0.16.0
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92
github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1
github.com/authzed/spicedb v1.29.5
github.com/brianvoe/gofakeit/v6 v6.23.2
Expand Down
4 changes: 2 additions & 2 deletions e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ github.com/Masterminds/squirrel v1.5.4 h1:uUcX/aBc8O7Fg9kaISIUsHXdKuqehiXAMQTYX8
github.com/Masterminds/squirrel v1.5.4/go.mod h1:NNaOrjSoIDfDA40n7sr2tPNZRfjzjA400rg+riTZj10=
github.com/antlr4-go/antlr/v4 v4.13.0 h1:lxCg3LAv+EUK6t1i0y1V6/SLeUi0eKEKdhQAlS8TVTI=
github.com/antlr4-go/antlr/v4 v4.13.0/go.mod h1:pfChB/xh/Unjila75QW7+VU4TSnWnnk9UTnmpPaOR2g=
github.com/authzed/authzed-go v0.16.0 h1:QzIduF472qZcHeEraNPSpnLFFV25ZQv6u7oDufvBf3o=
github.com/authzed/authzed-go v0.16.0/go.mod h1:Cx1DQKMX38u2fFVLZiGUuZnbTo2J7LCZEXmVak+TMak=
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92 h1:9BQQO4ga8naS5bl2aWeRoyF54Bxp1H28WMr+/XU7rLE=
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92/go.mod h1:Cx1DQKMX38u2fFVLZiGUuZnbTo2J7LCZEXmVak+TMak=
github.com/authzed/cel-go v0.20.2 h1:GlmLecGry7Z8HU0k+hmaHHUV05ZHrsFxduXHtIePvck=
github.com/authzed/cel-go v0.20.2/go.mod h1:pJHVFWbqUHV1J+klQoZubdKswlbxcsbojda3mye9kiU=
github.com/authzed/grpcutil v0.0.0-20240123092924-129dc0a6a6e1 h1:zBfQzia6Hz45pJBeURTrv1b6HezmejB6UmiGuBilHZM=
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ require (
github.com/IBM/pgxpoolprometheus v1.1.1
github.com/KimMachineGun/automemlimit v0.6.1
github.com/Masterminds/squirrel v1.5.4
github.com/authzed/authzed-go v0.16.0
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92

// NOTE: We are using a *copy* of `cel-go` here to ensure there isn't a conflict
// with the version used in Kubernetes. This is a temporary measure until we can
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -706,8 +706,8 @@ github.com/ashanbrown/forbidigo v1.6.0 h1:D3aewfM37Yb3pxHujIPSpTf6oQk9sc9WZi8ger
github.com/ashanbrown/forbidigo v1.6.0/go.mod h1:Y8j9jy9ZYAEHXdu723cUlraTqbzjKF1MUyfOKL+AjcU=
github.com/ashanbrown/makezero v1.1.1 h1:iCQ87C0V0vSyO+M9E/FZYbu65auqH0lnsOkf5FcB28s=
github.com/ashanbrown/makezero v1.1.1/go.mod h1:i1bJLCRSCHOcOa9Y6MyF2FTfMZMFdHvxKHxgO5Z1axI=
github.com/authzed/authzed-go v0.16.0 h1:QzIduF472qZcHeEraNPSpnLFFV25ZQv6u7oDufvBf3o=
github.com/authzed/authzed-go v0.16.0/go.mod h1:Cx1DQKMX38u2fFVLZiGUuZnbTo2J7LCZEXmVak+TMak=
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92 h1:9BQQO4ga8naS5bl2aWeRoyF54Bxp1H28WMr+/XU7rLE=
github.com/authzed/authzed-go v0.16.1-0.20241001202507-27cc182a7b92/go.mod h1:Cx1DQKMX38u2fFVLZiGUuZnbTo2J7LCZEXmVak+TMak=
github.com/authzed/cel-go v0.20.2 h1:GlmLecGry7Z8HU0k+hmaHHUV05ZHrsFxduXHtIePvck=
github.com/authzed/cel-go v0.20.2/go.mod h1:pJHVFWbqUHV1J+klQoZubdKswlbxcsbojda3mye9kiU=
github.com/authzed/consistent v0.1.0 h1:tlh1wvKoRbjRhMm2P+X5WQQyR54SRoS4MyjLOg17Mp8=
Expand Down
34 changes: 34 additions & 0 deletions internal/services/v1/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,3 +475,37 @@ func defaultIfZero[T comparable](value T, defaultValue T) T {
}
return value
}

// ErrTransactionMetadataTooLarge indicates that the metadata for a transaction is too large.
type ErrTransactionMetadataTooLarge struct {
error
metadataSize int
maxSize int
}

// NewTransactionMetadataTooLargeErr constructs a new transaction metadata too large error.
func NewTransactionMetadataTooLargeErr(metadataSize int, maxSize int) ErrTransactionMetadataTooLarge {
return ErrTransactionMetadataTooLarge{
error: fmt.Errorf("metadata size of %d is greater than maximum allowed of %d", metadataSize, maxSize),
metadataSize: metadataSize,
maxSize: maxSize,
}
}

func (err ErrTransactionMetadataTooLarge) MarshalZerologObject(e *zerolog.Event) {
e.Err(err.error).Int("metadataSize", err.metadataSize).Int("maxSize", err.maxSize)
}

func (err ErrTransactionMetadataTooLarge) GRPCStatus() *status.Status {
return spiceerrors.WithCodeAndDetails(
err,
codes.InvalidArgument,
spiceerrors.ForReason(
v1.ErrorReason_ERROR_REASON_TRANSACTION_METADATA_TOO_LARGE,
map[string]string{
"metadata_byte_size": strconv.Itoa(err.metadataSize),
"maximum_allowed_metadata_byte_size": strconv.Itoa(err.maxSize),
},
),
)
}
32 changes: 30 additions & 2 deletions internal/services/v1/relationships.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/prometheus/client_golang/prometheus/promauto"
"go.opentelemetry.io/otel/trace"
"google.golang.org/protobuf/proto"
"google.golang.org/protobuf/types/known/structpb"

"github.com/authzed/spicedb/internal/dispatch"
"github.com/authzed/spicedb/internal/middleware"
Expand Down Expand Up @@ -42,6 +43,8 @@ var writeUpdateCounter = promauto.NewHistogramVec(prometheus.HistogramOpts{
Buckets: []float64{0, 1, 2, 5, 10, 15, 25, 50, 100, 250, 500, 1000},
}, []string{"kind"})

const MaximumTransactionMetadataSize = 65000 // bytes. Limited by the BLOB size used in MySQL driver

// PermissionsServerConfig is configuration for the permissions server.
type PermissionsServerConfig struct {
// MaxUpdatesPerWrite holds the maximum number of updates allowed per
Expand Down Expand Up @@ -273,6 +276,10 @@ func (ps *permissionServer) ReadRelationships(req *v1.ReadRelationshipsRequest,
}

func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.WriteRelationshipsRequest) (*v1.WriteRelationshipsResponse, error) {
if err := ps.validateTransactionMetadata(req.OptionalTransactionMetadata); err != nil {
return nil, ps.rewriteError(ctx, err)
}

ds := datastoremw.MustFromContext(ctx)

span := trace.SpanFromContext(ctx)
Expand Down Expand Up @@ -347,7 +354,7 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ

span.AddEvent("write relationships")
return rwt.WriteRelationships(ctx, tupleUpdates)
})
}, options.WithMetadata(req.OptionalTransactionMetadata))
if err != nil {
return nil, ps.rewriteError(ctx, err)
}
Expand All @@ -367,7 +374,28 @@ func (ps *permissionServer) WriteRelationships(ctx context.Context, req *v1.Writ
}, nil
}

func (ps *permissionServer) validateTransactionMetadata(metadata *structpb.Struct) error {
if metadata == nil {
return nil
}

b, err := metadata.MarshalJSON()
if err != nil {
return err
}

if len(b) > MaximumTransactionMetadataSize {
return NewTransactionMetadataTooLargeErr(len(b), MaximumTransactionMetadataSize)
}

return nil
}

func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.DeleteRelationshipsRequest) (*v1.DeleteRelationshipsResponse, error) {
if err := ps.validateTransactionMetadata(req.OptionalTransactionMetadata); err != nil {
return nil, ps.rewriteError(ctx, err)
}

if len(req.OptionalPreconditions) > int(ps.config.MaxPreconditionsCount) {
return nil, ps.rewriteError(
ctx,
Expand Down Expand Up @@ -456,7 +484,7 @@ func (ps *permissionServer) DeleteRelationships(ctx context.Context, req *v1.Del
// Otherwise, kick off an unlimited deletion.
_, err = rwt.DeleteRelationships(ctx, req.RelationshipFilter)
return err
})
}, options.WithMetadata(req.OptionalTransactionMetadata))
if err != nil {
return nil, ps.rewriteError(ctx, err)
}
Expand Down
108 changes: 108 additions & 0 deletions internal/services/v1/relationships_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"io"
"maps"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -1344,6 +1345,113 @@ func TestDeleteRelationshipsPreconditionsOverLimit(t *testing.T) {
require.Contains(err.Error(), "precondition count of 2 is greater than maximum allowed of 1")
}

func TestWriteRelationshipsWithMetadata(t *testing.T) {
require := require.New(t)
conn, cleanup, _, beforeWriteRev := testserver.NewTestServerWithConfig(
require,
testTimedeltas[0],
memdb.DisableGC,
true,
testserver.ServerConfig{
MaxPreconditionsCount: 10,
MaxUpdatesPerWrite: 10,
},
tf.StandardDatastoreWithData,
)
t.Cleanup(cleanup)

client := v1.NewPermissionsServiceClient(conn)

metadata, err := structpb.NewStruct(map[string]any{
"foo": "123546",
})
require.NoError(err)

_, err = client.WriteRelationships(context.Background(), &v1.WriteRelationshipsRequest{
OptionalTransactionMetadata: metadata,
Updates: []*v1.RelationshipUpdate{
{
Operation: v1.RelationshipUpdate_OPERATION_TOUCH,
Relationship: rel("document", "newdoc", "parent", "folder", "afolder", ""),
},
},
})

require.NoError(err)

beforeWriteToken := zedtoken.MustNewFromRevision(beforeWriteRev)
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()

watchClient := v1.NewWatchServiceClient(conn)

stream, err := watchClient.Watch(ctx, &v1.WatchRequest{OptionalStartCursor: beforeWriteToken})
require.NoError(err)

resp, err := stream.Recv()
require.NoError(err)
require.Equal(metadata, resp.OptionalTransactionMetadata)
}

func TestWriteRelationshipsMetadataOverLimit(t *testing.T) {
require := require.New(t)
conn, cleanup, _, _ := testserver.NewTestServerWithConfig(
require,
testTimedeltas[0],
memdb.DisableGC,
true,
testserver.ServerConfig{
MaxPreconditionsCount: 1,
MaxUpdatesPerWrite: 1,
},
tf.StandardDatastoreWithData,
)
client := v1.NewPermissionsServiceClient(conn)
t.Cleanup(cleanup)

metadata, err := structpb.NewStruct(map[string]any{
"foo": strings.Repeat("hithere", 65000),
})
require.NoError(err)

_, err = client.WriteRelationships(context.Background(), &v1.WriteRelationshipsRequest{
OptionalTransactionMetadata: metadata,
})

require.Error(err)
require.Contains(err.Error(), "metadata size of 455010 is greater than maximum allowed of 65000")
}

func TestDeleteRelationshipsMetadataOverLimit(t *testing.T) {
require := require.New(t)
conn, cleanup, _, _ := testserver.NewTestServerWithConfig(
require,
testTimedeltas[0],
memdb.DisableGC,
true,
testserver.ServerConfig{
MaxPreconditionsCount: 1,
MaxUpdatesPerWrite: 1,
},
tf.StandardDatastoreWithData,
)
client := v1.NewPermissionsServiceClient(conn)
t.Cleanup(cleanup)

metadata, err := structpb.NewStruct(map[string]any{
"foo": strings.Repeat("hithere", 65000),
})
require.NoError(err)

_, err = client.DeleteRelationships(context.Background(), &v1.DeleteRelationshipsRequest{
OptionalTransactionMetadata: metadata,
RelationshipFilter: &v1.RelationshipFilter{},
})

require.Error(err)
require.Contains(err.Error(), "metadata size of 455010 is greater than maximum allowed of 65000")
}

func TestWriteRelationshipsPreconditionsOverLimit(t *testing.T) {
require := require.New(t)
conn, cleanup, _, _ := testserver.NewTestServerWithConfig(
Expand Down
5 changes: 3 additions & 2 deletions internal/services/v1/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ func (ws *watchServer) Watch(req *v1.WatchRequest, stream v1.WatchService_WatchS
filtered := filterUpdates(objectTypes, filters, update.RelationshipChanges)
if len(filtered) > 0 {
if err := stream.Send(&v1.WatchResponse{
Updates: filtered,
ChangesThrough: zedtoken.MustNewFromRevision(update.Revision),
Updates: filtered,
ChangesThrough: zedtoken.MustNewFromRevision(update.Revision),
OptionalTransactionMetadata: update.Metadata,
}); err != nil {
return status.Errorf(codes.Canceled, "watch canceled by user: %s", err)
}
Expand Down

0 comments on commit bb6362c

Please sign in to comment.