diff --git a/cmd/serve.go b/cmd/serve.go index 0f8650854..92769298e 100644 --- a/cmd/serve.go +++ b/cmd/serve.go @@ -12,6 +12,8 @@ import ( "syscall" "time" + "golang.org/x/exp/slices" + "github.com/jackc/pgx/v4" "github.com/stripe/stripe-go/v79" @@ -331,7 +333,17 @@ func buildAPIDependencies( namespaceService := namespace.NewService(namespaceRepository) authzSchemaRepository := spicedb.NewSchemaRepository(logger, sdb) - authzRelationRepository := spicedb.NewRelationRepository(sdb, cfg.SpiceDB.FullyConsistent, cfg.SpiceDB.CheckTrace) + consistencyLevel := spicedb.ConsistencyLevel(cfg.SpiceDB.Consistency) + if cfg.SpiceDB.FullyConsistent { + consistencyLevel = spicedb.ConsistencyLevelFull + } + if !slices.Contains([]spicedb.ConsistencyLevel{ + spicedb.ConsistencyLevelFull, + spicedb.ConsistencyLevelBestEffort, + spicedb.ConsistencyLevelMinimizeLatency}, consistencyLevel) { + return api.Deps{}, fmt.Errorf("invalid consistency level: %s", consistencyLevel) + } + authzRelationRepository := spicedb.NewRelationRepository(sdb, consistencyLevel, cfg.SpiceDB.CheckTrace) permissionRepository := postgres.NewPermissionRepository(dbc) permissionService := permission.NewService(permissionRepository) diff --git a/config/config_test.go b/config/config_test.go index b0be39c4a..ecbe0752f 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -38,6 +38,7 @@ func TestLoad(t *testing.T) { Host: "spicedb.localhost", Port: "50051", PreSharedKey: "randomkey", + Consistency: spicedb.ConsistencyLevelBestEffort.String(), }, }, wantErr: false, diff --git a/config/sample.config.yaml b/config/sample.config.yaml index 9a4fbe010..732e4acd5 100644 --- a/config/sample.config.yaml +++ b/config/sample.config.yaml @@ -160,12 +160,15 @@ spicedb: host: spicedb.localhost pre_shared_key: randomkey port: 50051 - # fully_consistent ensures APIs although slower than usual will result in responses always most consistent - # suggested to keep it false for performance - fully_consistent: false # check_trace enables tracing in check api for spicedb, it adds considerable # latency to the check calls and shouldn't be enabled in production check_trace: false + # consistency ensures Authz server consistency guarantees for various operations + # Possible values are: + # - "full": Guarantees that the data is always fresh although API calls might be slower than usual + # - "best_effort": Guarantees that the data is the best effort fresh [default] + # - "minimize_latency": Tries to prioritise minimal latency + consistency: "best_effort" billing: # stripe key to be used for billing diff --git a/docs/docs/reference/configurations.md b/docs/docs/reference/configurations.md index b2a0e20ef..903ad19cf 100644 --- a/docs/docs/reference/configurations.md +++ b/docs/docs/reference/configurations.md @@ -155,9 +155,12 @@ spicedb: host: spicedb.localhost pre_shared_key: randomkey port: 50051 - # fully_consistent ensures APIs although slower than usual will result in responses always most consistent - # suggested to keep it false for performance - fully_consistent: false + # consistency ensures Authz server consistency guarantees for various operations + # Possible values are: + # - "full": Guarantees that the data is always fresh although API calls might be slower than usual + # - "best_effort": Guarantees that the data is the best effort fresh [default] + # - "minimize_latency": Tries to prioritise minimal latency + consistency: "best_effort" # check_trace enables tracing in check api for spicedb, it adds considerable # latency to the check calls and shouldn't be enabled in production check_trace: false diff --git a/internal/store/spicedb/config.go b/internal/store/spicedb/config.go index 5e327caba..a20662b5f 100644 --- a/internal/store/spicedb/config.go +++ b/internal/store/spicedb/config.go @@ -5,9 +5,17 @@ type Config struct { Port string `yaml:"port" default:"50051"` PreSharedKey string `yaml:"pre_shared_key" mapstructure:"pre_shared_key"` + // Deprecated: Use Consistency instead // FullyConsistent ensures APIs although slower than usual will result in responses always most consistent FullyConsistent bool `yaml:"fully_consistent" mapstructure:"fully_consistent" default:"false"` + // Consistency ensures Authz server consistency guarantees for various operations + // Possible values are: + // - "full": Guarantees that the data is always fresh + // - "best_effort": Guarantees that the data is the best effort fresh + // - "minimize_latency": Tries to prioritise minimal latency + Consistency string `yaml:"consistency" mapstructure:"consistency" default:"best_effort"` + // CheckTrace enables tracing in check api for spicedb, it adds considerable // latency to the check calls and shouldn't be enabled in production CheckTrace bool `yaml:"check_trace" mapstructure:"check_trace" default:"false"` diff --git a/internal/store/spicedb/relation_repository.go b/internal/store/spicedb/relation_repository.go index f2e4fdb84..9de63a265 100644 --- a/internal/store/spicedb/relation_repository.go +++ b/internal/store/spicedb/relation_repository.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "sync/atomic" grpczap "github.com/grpc-ecosystem/go-grpc-middleware/logging/zap/ctxzap" "go.uber.org/zap" @@ -18,29 +19,48 @@ import ( type RelationRepository struct { spiceDB *SpiceDB - // fullyConsistent makes sure all APIs are highly consistent on their responses - // turning it on will result in slower API calls but useful in tests - fullyConsistent bool + // Consistency ensures Authz server consistency guarantees for various operations + // Possible values are: + // - "full": Guarantees that the data is always fresh + // - "best_effort": Guarantees that the data is the best effort fresh + // - "minimize_latency": Tries to prioritise minimal latency + consistency ConsistencyLevel // tracing enables debug traces for check calls tracing bool - // TODO(kushsharma): after every call, check if the response returns a relationship - // snapshot(zedtoken/zookie), if it does, store it in a cache/db, and use it for subsequent calls - // this will make the calls faster and avoid the use of fully consistent spiceDB + // lastToken is the last zookie returned by the server, this is cached at instance level and + // maybe not be consistent across multiple instances but that is fine in most cases as + // the token is only used in lookup or list calls, for permission checks we always use the + // consistency level. Storing it in a shared db/cache will make it consistent across instances. + // We can also store multiple tokens in the cache based on what kind of resource we are dealing with + // but that adds complexity. + lastToken atomic.Pointer[authzedpb.ZedToken] } +type ConsistencyLevel string + +func (c ConsistencyLevel) String() string { + return string(c) +} + +const ( + ConsistencyLevelFull ConsistencyLevel = "full" + ConsistencyLevelBestEffort ConsistencyLevel = "best_effort" + ConsistencyLevelMinimizeLatency ConsistencyLevel = "minimize_latency" +) + const nrProductName = "spicedb" -func NewRelationRepository(spiceDB *SpiceDB, fullyConsistent bool, tracing bool) *RelationRepository { +func NewRelationRepository(spiceDB *SpiceDB, consistency ConsistencyLevel, tracing bool) *RelationRepository { return &RelationRepository{ - spiceDB: spiceDB, - fullyConsistent: fullyConsistent, - tracing: tracing, + spiceDB: spiceDB, + consistency: consistency, + tracing: tracing, } } -func (r RelationRepository) Add(ctx context.Context, rel relation.Relation) error { +func (r *RelationRepository) Add(ctx context.Context, rel relation.Relation) error { relationship := &authzedpb.Relationship{ Resource: &authzedpb.ObjectReference{ ObjectType: rel.Object.Namespace, @@ -79,16 +99,18 @@ func (r RelationRepository) Add(ctx context.Context, rel relation.Relation) erro defer nr.End() } - if _, err := r.spiceDB.client.WriteRelationships(ctx, request); err != nil { + resp, err := r.spiceDB.client.WriteRelationships(ctx, request) + if err != nil { return err } + r.lastToken.Store(resp.GetWrittenAt()) return nil } -func (r RelationRepository) Check(ctx context.Context, rel relation.Relation) (bool, error) { +func (r *RelationRepository) Check(ctx context.Context, rel relation.Relation) (bool, error) { request := &authzedpb.CheckPermissionRequest{ - Consistency: r.getConsistency(), + Consistency: r.getConsistencyForCheck(), Resource: &authzedpb.ObjectReference{ ObjectId: rel.Object.ID, ObjectType: rel.Object.Namespace, @@ -124,10 +146,11 @@ func (r RelationRepository) Check(ctx context.Context, rel relation.Relation) (b grpczap.Extract(ctx).Info("CheckPermission", zap.String("trace", string(str))) } + r.lastToken.Store(response.GetCheckedAt()) return response.GetPermissionship() == authzedpb.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION, nil } -func (r RelationRepository) Delete(ctx context.Context, rel relation.Relation) error { +func (r *RelationRepository) Delete(ctx context.Context, rel relation.Relation) error { if rel.Object.Namespace == "" { return errors.New("object namespace is required to delete a relation") } @@ -160,15 +183,16 @@ func (r RelationRepository) Delete(ctx context.Context, rel relation.Relation) e } defer nr.End() } - _, err := r.spiceDB.client.DeleteRelationships(ctx, request) + resp, err := r.spiceDB.client.DeleteRelationships(ctx, request) if err != nil { return err } + r.lastToken.Store(resp.GetDeletedAt()) return nil } -func (r RelationRepository) LookupSubjects(ctx context.Context, rel relation.Relation) ([]string, error) { +func (r *RelationRepository) LookupSubjects(ctx context.Context, rel relation.Relation) ([]string, error) { resp, err := r.spiceDB.client.LookupSubjects(ctx, &authzedpb.LookupSubjectsRequest{ Consistency: r.getConsistency(), Resource: &authzedpb.ObjectReference{ @@ -195,7 +219,7 @@ func (r RelationRepository) LookupSubjects(ctx context.Context, rel relation.Rel return subjects, nil } -func (r RelationRepository) LookupResources(ctx context.Context, rel relation.Relation) ([]string, error) { +func (r *RelationRepository) LookupResources(ctx context.Context, rel relation.Relation) ([]string, error) { resp, err := r.spiceDB.client.LookupResources(ctx, &authzedpb.LookupResourcesRequest{ Consistency: r.getConsistency(), ResourceObjectType: rel.Object.Namespace, @@ -226,7 +250,7 @@ func (r RelationRepository) LookupResources(ctx context.Context, rel relation.Re } // ListRelations shouldn't be used in high TPS flows as consistency requirements are set high -func (r RelationRepository) ListRelations(ctx context.Context, rel relation.Relation) ([]relation.Relation, error) { +func (r *RelationRepository) ListRelations(ctx context.Context, rel relation.Relation) ([]relation.Relation, error) { resp, err := r.spiceDB.client.ReadRelationships(ctx, &authzedpb.ReadRelationshipsRequest{ Consistency: r.getConsistency(), RelationshipFilter: &authzedpb.RelationshipFilter{ @@ -268,14 +292,7 @@ func (r RelationRepository) ListRelations(ctx context.Context, rel relation.Rela return rels, nil } -func (r RelationRepository) getConsistency() *authzedpb.Consistency { - if !r.fullyConsistent { - return nil - } - return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_FullyConsistent{FullyConsistent: true}} -} - -func (r RelationRepository) BatchCheck(ctx context.Context, relations []relation.Relation) ([]relation.CheckPair, error) { +func (r *RelationRepository) BatchCheck(ctx context.Context, relations []relation.Relation) ([]relation.CheckPair, error) { result := make([]relation.CheckPair, len(relations)) items := make([]*authzedpb.BulkCheckPermissionRequestItem, 0, len(relations)) for _, rel := range relations { @@ -295,7 +312,7 @@ func (r RelationRepository) BatchCheck(ctx context.Context, relations []relation }) } request := &authzedpb.BulkCheckPermissionRequest{ - Consistency: r.getConsistency(), + Consistency: r.getConsistencyForCheck(), Items: items, } @@ -329,5 +346,33 @@ func (r RelationRepository) BatchCheck(ctx context.Context, relations []relation result[itemIdx].Status = item.GetItem().GetPermissionship() == authzedpb.CheckPermissionResponse_PERMISSIONSHIP_HAS_PERMISSION } } + + r.lastToken.Store(response.GetCheckedAt()) return result, respErr } + +func (r *RelationRepository) getConsistency() *authzedpb.Consistency { + switch r.consistency { + case ConsistencyLevelMinimizeLatency: + return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_MinimizeLatency{MinimizeLatency: true}} + case ConsistencyLevelFull: + return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_FullyConsistent{FullyConsistent: true}} + } + + lastToken := r.lastToken.Load() + if lastToken == nil { + return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_FullyConsistent{FullyConsistent: true}} + } + return &authzedpb.Consistency{ + Requirement: &authzedpb.Consistency_AtLeastAsFresh{ + AtLeastAsFresh: lastToken, + }, + } +} + +func (r *RelationRepository) getConsistencyForCheck() *authzedpb.Consistency { + if r.consistency == ConsistencyLevelMinimizeLatency { + return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_MinimizeLatency{MinimizeLatency: true}} + } + return &authzedpb.Consistency{Requirement: &authzedpb.Consistency_FullyConsistent{FullyConsistent: true}} +} diff --git a/test/e2e/testbench/testbench.go b/test/e2e/testbench/testbench.go index 3865feb6f..c5553c593 100644 --- a/test/e2e/testbench/testbench.go +++ b/test/e2e/testbench/testbench.go @@ -90,10 +90,10 @@ func Init(appConfig *config.Frontier) (*TestBench, error) { MaxQueryTimeout: time.Second * 30, } appConfig.SpiceDB = spicedb.Config{ - Host: "localhost", - Port: spiceDBPort, - PreSharedKey: preSharedKey, - FullyConsistent: true, + Host: "localhost", + Port: spiceDBPort, + PreSharedKey: preSharedKey, + Consistency: spicedb.ConsistencyLevelBestEffort.String(), } appConfig.App.Admin.Users = []string{OrgAdminEmail} appConfig.App.Webhook.EncryptionKey = "kmm4ECoWU21K2ZoyTcYLd6w7DfhoUoap"