caveat in MemDB datastore #807

vroldanbet · 2022-09-08T14:58:57Z

This PR introduces datastore interface to do CRUD operations with Caveats. We choose to start with MemDB because it does not require migrations, it's not meant to be used in production, and thus allows us to start building the graph evaluation logic without requiring modifications to all datastores. Once we've nailed its usage, we can go back to add support in the rest of datastores.

The main design choices are:

Caveats have its own lifecycle independent from Namespaces
They get a globally unique identifier so that we can continue referencing them even though name may change
Name is a user-friendly way to reference a caveat. It will be used in the schema language to give caveats a name the schema developer can reference in their definition
The datastore does not know anything about how caveats are implemented, and are simply stored as a sequence of bytes
Tuples will reference caveats by its ID.
Tuples may include a caveat context, which will be passed along to the caveat evaluator. Think input arguments of a function.
The caveat package has evolved to serialize the caveat name with the proto representation. This is necessary for type checking in the schema.
New interfaces are added to make datastores support caveats. They include the various CRUD operations. Writes are upserts, and caveats can be looked up by ID or name.
Two caveats with the same name are considered the same caveat. There cannot be two caveats with distinct IDs and same Name

internal/datastore/memdb/caveat.go

internal/datastore/memdb/readwrite.go

pkg/caveats/compile.go

internal/datastore/memdb/readwrite.go

internal/datastore/memdb/schema.go

instead of using protobuf.Any, we use protobuf.Struct that better maps a map[string]any we move away from the concept of caveat digest and will reference them by name. Anonymous caveats will have a globally-unique name that may likely be the result of performing a digest of the signature and payload we replace "caveat logic" with "caveat expression" we also change the term "predefined variables" with "caveat context" to refer to the state persisted alongside the tuple. these are variables that will be injected at caveat evaluation time as a minor inconvenience the relationship.RelationTuple method now has to return an error, propagated from the call to structpb.NewStruct. That led to changes in all callsites using it.

@josephschorr

h/t @josephschorr

cavat names are not a good source for database ID. The ID will be used as the foreign key used in in tuples to keep track of caveats.

we are going to punt on anonymous caveats for now as we've found named caveats can fulfil most requirements we've stumbled upon as a consequence the caveat reference is simplified and now we use the caveat id if we ever are to introduce anonymous caveats, we can do a proto oneof between a named caveat ID and an anonymous caveat payload

I couldn't determine why go-memdb does not honor secondary index uniqueness, so I added a safeguard.

this commit removes validating that a caveated tuple references an existing caveat. The controller layer is responsible to maintain the integrity. It also helps avoid duplicated validation across layers

if was formerly named "CaveatReference" in the proto def

internal/datastore/memdb/caveat.go

internal/datastore/memdb/caveat_test.go

pkg/datastore/caveat.go

proto/internal/core/v1/core.proto

josephschorr · 2022-09-19T17:16:56Z

proto/internal/core/v1/core.proto

+    max_bytes : 128,
+  } ];
+
+  /** expression is the byte representation of a caveat's logic */


I wonder if we should just make this DecodedCaveat and move it in here...

honestly it's not clear to me what is the design rationale behind the impl proto package. It feels like both would overlap in responsibility.

this is important in order to provide a good experience in dev tooling

for safety until those datastores implement caveat support

pkg/datastore/caveat.go

proto/internal/core/v1/core.proto

internal/datastore/memdb/caveat.go

internal/datastore/mysql/readwrite.go

internal/datastore/crdb/readwrite.go

in order to demonstrate snapshot reads the caveat write operation had to be adjusted to support upserts. Thus it's no longer returning an error on duplicate caveats

josephschorr

LGTM

github-actions bot added area/datastore Affects the storage system area/tooling Affects the dev or user toolchain (e.g. tests, ci, build tools) labels Sep 8, 2022

vroldanbet force-pushed the caveat-hybrid-spike branch 3 times, most recently from 4edd111 to dff6f68 Compare September 13, 2022 14:24

github-actions bot added the area/dependencies Affects dependencies label Sep 13, 2022

vroldanbet force-pushed the caveat-hybrid-spike branch 3 times, most recently from 8951238 to 2efc7c2 Compare September 13, 2022 15:02

vroldanbet requested a review from josephschorr September 14, 2022 14:26

josephschorr reviewed Sep 14, 2022

View reviewed changes

vroldanbet force-pushed the caveat-hybrid-spike branch 3 times, most recently from d9bea35 to 30d445d Compare September 15, 2022 18:29

vroldanbet changed the title ~~spiking named/anonymous caveat design~~ caveat in datastore redux Sep 15, 2022

vroldanbet force-pushed the caveat-hybrid-spike branch from 015507d to a972c8e Compare September 15, 2022 19:05

vroldanbet requested a review from josephschorr September 15, 2022 19:14

vroldanbet self-assigned this Sep 15, 2022

josephschorr reviewed Sep 16, 2022

View reviewed changes

internal/datastore/memdb/readwrite.go Outdated Show resolved Hide resolved

internal/datastore/memdb/schema.go Outdated Show resolved Hide resolved

vroldanbet added 11 commits September 19, 2022 12:35

spike named/anonymous caveats

1ac4f6a

pluralize interface methods

048999e

h/t @josephschorr

introduce Caveat ID

44b8f10

cavat names are not a good source for database ID. The ID will be used as the foreign key used in in tuples to keep track of caveats.

uses uint64 as caveatID instead of string

a20ccbb

test name uniqueness

0762676

I couldn't determine why go-memdb does not honor secondary index uniqueness, so I added a safeguard.

add interface docs

0c9112f

serialize name with caveat and test

e6aafb2

datastore no longer responsible to check if caveat exists

bf3245d

this commit removes validating that a caveated tuple references an existing caveat. The controller layer is responsible to maintain the integrity. It also helps avoid duplicated validation across layers

rename internal memdb struct to align with proto name

8a6548b

if was formerly named "CaveatReference" in the proto def

vroldanbet force-pushed the caveat-hybrid-spike branch from 77b11ff to 8a6548b Compare September 19, 2022 11:36

github-actions bot removed the area/dependencies Affects dependencies label Sep 19, 2022

vroldanbet marked this pull request as ready for review September 19, 2022 11:38

vroldanbet requested a review from a team September 19, 2022 11:38

generate with recently introduced vtprotobuf

cc5b831

vroldanbet changed the title ~~caveat in datastore redux~~ caveat in MemDB datastore redux Sep 19, 2022

josephschorr reviewed Sep 19, 2022

View reviewed changes

vroldanbet added 3 commits September 19, 2022 18:50

address review comments by @josephschorr

44e0782

add custom errors for not found name/id

3ff4020

this is important in order to provide a good experience in dev tooling

panic if caveats are attempted to be written

9a702d6

for safety until those datastores implement caveat support

josephschorr reviewed Sep 20, 2022

View reviewed changes

vroldanbet added 3 commits September 20, 2022 08:41

clarify in comment MemDB does not enforce uniqueness

d104b71

more cosmetic changes from PR review

1282eac

demonstrate snapshot reads for memdb

b22266d

in order to demonstrate snapshot reads the caveat write operation had to be adjusted to support upserts. Thus it's no longer returning an error on duplicate caveats

josephschorr approved these changes Sep 20, 2022

View reviewed changes

vroldanbet merged commit bb5f08b into authzed:main Sep 20, 2022

vroldanbet deleted the caveat-hybrid-spike branch September 20, 2022 16:57

github-actions bot locked and limited conversation to collaborators Sep 20, 2022

vroldanbet changed the title ~~caveat in MemDB datastore redux~~ caveat in MemDB datastore Sep 21, 2022

jzelinskie added the area/caveats Affects caveated relationships label Oct 8, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

caveat in MemDB datastore #807

caveat in MemDB datastore #807

vroldanbet commented Sep 8, 2022 •

edited

Loading

josephschorr Sep 19, 2022

vroldanbet Sep 19, 2022

josephschorr left a comment

caveat in MemDB datastore #807

caveat in MemDB datastore #807

Conversation

vroldanbet commented Sep 8, 2022 • edited Loading

josephschorr Sep 19, 2022

Choose a reason for hiding this comment

vroldanbet Sep 19, 2022

Choose a reason for hiding this comment

josephschorr left a comment

Choose a reason for hiding this comment

vroldanbet commented Sep 8, 2022 •

edited

Loading