jamar-criteo
commented
2 months ago Hello @aaguiarz ,
You identified quite a few solutions to support modularity in openFGA to modularize Datastore adapters (and, as you mentioned, each of them has some drawbacks).
Keeping each adapter in its own repository and bundling them in the binary. We can define clear ownership for each repository and warn the community about the level of support of each adapter. It could happen that a newer version of OpenFGA does not support a specific database adapter if the interface changed and the adapter was not updated.
This solution might be suitable, but it will raise a few questions about code ownership boundaries (what code belongs to what repository), which I will develop further.
Use Go Plugins as a way to load storage adapters, so they are maintained in their own repository and not bundled with the binary. This approach has several drawbacks, discussed in the Go Plugins documentation linked above.
As mentioned in the provided link above, the warning list of the documentation is not attractive. (see: https://pkg.go.dev/plugin#hdr-Warnings)
Make OpenFGA communicate with storage adapters using gRPC. This adds complexity + performance overhead.
As performance is a strong requirement for many organizations, adding additional latency might be a strong blocker for a lot of actors in the industry, as they expect permission checks to be fast. (especially in micro-service-oriented architecture, where authorization checks can be performed at different levels)
When you mentioned this roadmap issue on Slack, I came back to my review (to support MSSQL), and tried to understand what needs to be externalized to be modular enough.
migration files
In term of ownership, I feel that each adapter should own their own migration files.
That said, the current implementation expects migration files to be locally available next to the openfga binary (in the assets/migrations/{adapter} folder).
For this reason, the migration logic needs to be adjusted to consider that migration files would not be part of openFGA core anymore. (NOTE: maybe this can also be leveraged at the build/release time).
connection string parsing logic
The connection string logic is kind of duplicated between the migration (cmd/migrate/migrate.go) & each adapter constructor (New method).
Since the parsing of the query differ from an adapter to another, I think this logic should be part of a dedicated go file (owned by the adapter owner).
testing logic (related to the adapter)
The testing logic at the adapter level, would be owned by the organization maintaining the connector.
the datastore engine's code
The datastore adapter could be declared in remote repository (equivalent of pkg/storage/{adapter}) along with its: connection string parser, tests and migration files. (as described above)
testing logic (related to openFGA, high level test / benchmarks)
Many high-level tests start: (1) an openFGA instance (2) a test container. Then, execute some queries against this instance.
Currently, those tests evaluate that openFGA still works against all supported datastore adapters.
I think that those tests should still exist and be maintained within openFGA core repository, since they would allow to detect any regression in supported adapters.
Makefile (make dev-run)
The way the Makefile is implemented for development purpose: it will start a container along with the openFGA instance.
This process requires to explicitly support a case, to start openFGA & its container, which makes a small code to maintain in openFGA core repository.
I am not sure this is worth the effort, but a possible alternative would be to support an additional parameter (like --local) to openFGA, which will rely on the adapter's implementation to start its own container (if needed).
That’s food for thought. I saw that this topic was identified as a topic for your internal Hackthon (next week). I would be glad to collaborate and share some information & details with the person in charge of the implementation, if more information is required.
jon-whit
Storage adapters that are contributed by the community currently become part of the OpenFGA repository + binary, and maintaining them becomes responsibility of the core OpenFGA team.
In order to support more database adapters while reducing the impact of supporting them for the OpenFGA team we can explore different options:
Keeping each adapter in its own repository, and bundling them in the binary. We can define clear ownership for each repository and warn the community about the level of support of each adapter. It could happen that a newer version of OpenFGA does not support a specific database adapter if the interface changed and the adapter was not updated.
Use Go Plugins as a way to load storage adapters, so they are maintained in their own repository and not bundled with the binary. This approach has several drawbacks, discussed in the Go Plugins documentation linked above.
Make OpenFGA communicate with storage adapters using gRPC. This adds complexity + performance overhead.
In any case, we'd need to improve our test suite to make sure storage adapters are conformant with the OpenFGA required behavior, so they are easier to contribute and maintain.