From bb6362c9038727c4f96d0efa2569920d32fce1a5 Mon Sep 17 00:00:00 2001 From: Joseph Schorr Date: Tue, 1 Oct 2024 17:07:32 -0400 Subject: [PATCH] Add API support for transaction metadata on WriteRels and DeleteRels --- e2e/go.mod | 2 +- e2e/go.sum | 4 +- go.mod | 2 +- go.sum | 4 +- internal/services/v1/errors.go | 34 +++++++ internal/services/v1/relationships.go | 32 +++++- internal/services/v1/relationships_test.go | 108 +++++++++++++++++++++ internal/services/v1/watch.go | 5 +- 8 files changed, 181 insertions(+), 10 deletions(-) diff --git a/e2e/go.mod b/e2e/go.mod index c9684a1c2e..2ccc06fa2d 100644 --- a/e2e/go.mod +++ b/e2e/go.mod @@ -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 diff --git a/e2e/go.sum b/e2e/go.sum index 049a6eb7be..044e469a37 100644 --- a/e2e/go.sum +++ b/e2e/go.sum @@ -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= diff --git a/go.mod b/go.mod index 7f4e085198..9408e3b806 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index cb3d8edb0e..39475ef4ea 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/internal/services/v1/errors.go b/internal/services/v1/errors.go index 866ab980e4..068de6de17 100644 --- a/internal/services/v1/errors.go +++ b/internal/services/v1/errors.go @@ -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), + }, + ), + ) +} diff --git a/internal/services/v1/relationships.go b/internal/services/v1/relationships.go index b8feb62b74..327c05bdc0 100644 --- a/internal/services/v1/relationships.go +++ b/internal/services/v1/relationships.go @@ -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" @@ -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 @@ -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) @@ -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) } @@ -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, @@ -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) } diff --git a/internal/services/v1/relationships_test.go b/internal/services/v1/relationships_test.go index de0d7ba1c9..dc2a50dbb5 100644 --- a/internal/services/v1/relationships_test.go +++ b/internal/services/v1/relationships_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "maps" + "strings" "testing" "time" @@ -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( diff --git a/internal/services/v1/watch.go b/internal/services/v1/watch.go index 0bb42fc733..45f53adcc4 100644 --- a/internal/services/v1/watch.go +++ b/internal/services/v1/watch.go @@ -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) }