Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: initial OpenFGA v2beta1 protobuf API development #127

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

jon-whit
Copy link
Member

@jon-whit jon-whit commented Dec 13, 2023

Description

Start the development branch for the v2beta1 protobuf API definitions for OpenFGA. This is to get things started with that new API development effort as we go into the new year.

As we work through these changes in the scope of this PR, lets try to focus on a few guiding principles:

  1. Well typed and strongly typed protobuf API contract - prefer objects over string encoded entities where you can (e.g. Object instead of string object)

  2. Separation of input types vs output types and a clear definition of required vs non-required fields within them- this really helps other development in our tooling such as in the SDKs etc..

  3. Think about the gRPC experience, not just the HTTP mapping - the protobuf API is the definitive source of truth. The HTTP API is derived off of it. When in doubt, the protobuf API should take precedence.

  4. Give a shit about naming things - if you've had a beef with the API names, speak up and fight for the cause 馃槃

  5. Encourage strong alignment with buf lint lint style guidelines so that we have a standard - we shouldn't have to make exceptions regularly for our protobuf API linting behavior. Stick to the community standard of using buf lint and the default rules for it.

  6. Consider Google API Improvement Proposals (AIPs) as a good starting point for inspiration behind certain API changes and design - we don't have to copy Google API guidelines, but they've clearly given a lot of API experience some thought and since they are the primary driving force behind gRPC and RPC APIs in general, their guidelines are a good reference point for ideas of achieving certain design goals. In particular, consider the AIPs around standard methods.

A few notable changes to get started:

  • The new API has been designed with an emphasis on a strongly typed protobuf API. For example, instead of string object fields we have explicit Object object fields.

  • User fields have been renamed to Subject to avoid confusion with the often defined type name user. Subject is a strongly typed entity that can be one_of Object (a direct object), TypedWildcard (to represent the public wildcard), or a SubjectSet (to represent the set of subjects that expand to one or more concrete objects). A SubjectSet models Usersets in Zanzibar nomenclature.

  • TupleKey has been more aptly named RelationshipTuple - this coincides with our official documentation Concepts better (see Relationship Tuple)

  • Userset, which previously represented a relation rewrite rule, has been refactored to a more aptly named RelationRewrite and the model of it is much more uniform.

  • The relations defined in an AuthorizationModel has changed from a direct Userset rewrite field to a map of Relation structures map<string, Relation>. This will make things more uniform by far in all application code that deals with an AuthorizationModel structure and will more easily allow for indexing into the model's relation map to grab a particular relation.

  • Relation now uniformly represents a relation definition, and the constructs associated with it such as the type restrictions that apply to it etc..

  • The ReadChanges API has been renamed to WatchChanges and has been relocated to an auxillary gRPC service definition WatchService. This will allow for a separate watch server to implement the WatchService and allow that service to be hosted and scaled independently of the main OpenFGAService which services Checks, ListObjects, etc.. The ability to serve and scale this service independently will be more important especially as we start building other features around changelog metadata such as the indexer and ExpandedWatchChanges endpoints etc.. These workloads will be different and need not be served from the same server in general..

References

Review Checklist

  • I have clicked on "allow edits by maintainers".
  • I have added documentation for new/changed functionality in this PR or in a PR to openfga.dev [Provide a link to any relevant PRs in the references section above]
  • The correct base branch is being used, if not main
  • I have added tests to validate that the change in functionality is working as expected

@jon-whit
Copy link
Member Author

  • todo: add protobuf doc to every protobuf message and field as appropriate.
  • todo: add required field specifications and other protobuf annotations for OpenAPI generation


import "google/protobuf/struct.proto";

message RelationshipTuple {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WHat do you think about naming them RelationshipTuple and RelationshipTupleWithCondition?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So invert them?

I think the API more unanymously operates on a RelationshipTuple which can optionally have a condition. For example the WriteRelationshipTuples API, the ReadRelationshipTuples API, etc.. The exception are those APIs that do not need a condition in the tuple representation whatsoever. So I think I lean toward leaving as is.

// restriction allows it.
message TypeRestriction {
oneof type_reference {
UnconditionedObjectTypeRestriction unconditioned_object_type_reference = 1;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the separation? also all can be conditioned not just object type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes the API more strongly typed. A type restriction can reference a direct object type with no condition or one explicitly with a condition. The later requires the condition to be defined, the former does not. To uniquely distinguish these cases we make the protobuf API strongly typed to represent the difference.


rpc WriteModel(WriteModelRequest) returns (WriteModelResponse);
rpc ReadModel(ReadModelRequest) returns (ReadModelResponse);
rpc ListModels(ListModelsRequest) returns (ListModelsResponse);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadChanges?
Read?
BatchCheck?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More to come 馃槃 . Just started with some.

openfga/v2beta1/openfga_service.proto Outdated Show resolved Hide resolved
string model_id = 2;
openfga.v2beta1.Subject subject = 3;
string relation = 4;
string object_type = 5;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, maybe:
object: {
type: ...
}

Copy link
Member Author

@jon-whit jon-whit Dec 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would imply another message definition of the form

message ObjectWithoutId {
    string type = 1;
}

That's a bit overloaded and adds message level indirection for little to no gain. string object_type points to the Object.Type field. Accepting the string is more straightforward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants