search

openfga / api

Protocol Buffers used by OpenFGA
Apache License 2.0
19 stars 14 forks source link

Breaking Change in Proto Field Ordering #126

Open alee792 opened 11 months ago

alee792 commented 11 months ago

This proto field re-ordering breaks compatibility. The user and object fields get swapped, and tuple checks are just entirely misinterpreted: https://github.com/openfga/api/commit/5daf658e21c2aa392532f7ab1e0549a877f48eeb#diff-2a88655b667aad16ec564eded7b5739e88e7d8b9da8a5231008519c3d3b80bb9L28

PR here: https://github.com/openfga/api/pull/97#

It looks like assertions were similarly affected and that change was reverted. Was there some kind of safe, backwards compatible migration that I missed? I would have assumed more people would be affected by this breaking change.

jon-whit commented 11 months ago

@alee792 releases v1.3.8 through v1.3.10 have been building up in preparation for a bigger feature release v1.4.0 which we just released today.

As part of this development we had to make some changes to the protobuf API to better model the changes we introduced to support a new feature we call Conditional Relationship Tuples. One of the most notable changes we made that had an impact pervasively throughout the whole protobuf API was the changes to TupleKey. TupleKey was overloaded in various usages across the API in a non-uniform way, and the addition of Conditions change its usage pattern a bit. To address this, we introduced a new protobuf message called TupleKeyWithoutCondition and we repurposed the TupleKey to contain an optional condition, because tuples can now be optionally conditioned upon something. To accommodate these changes in a way that we felt made the API more uniform we decided to make some changes across the board that has impact to both field level and message level breaking changes. Since we had decided to make message level breaking changes, which would impact compatibility with old clients anyways, we also decided to re-order the fields of the new message structure so that it better matched the order of our documentation.

We had intended to include these details about client compatibility in the v1.3.8 release notes, but it appears we failed to do so. I will be sure we get that retroactively updated and sincerely apologize for any disruption that may have caused. In the v1.4.0 release notes we issued a warning not to rollback to a release prior to v1.3.9 of OpenFGA if you've upgraded to v1.4.0 to avoid some side-effects, and part of that includes the incompatibilities between older clients and servers. I will make sure we update the documentation to reflect the concern with older grpc clients as well. Note that the OpenAPI (HTTP API) was not impacted, but gRPC clients were.

We will soon be releasing some official documentation on the new Conditional Relationship Tuples feature and as part of that documentation we will have a more detailed overview of the impact of upgrades from v1.3.7 to v1.3.8 and up to v1.4.0.

Our guidance/advise at this time would be either a) stay on v1.3.7 and use a gRPC client that references the older protobuf definition(s) until you can upgrade client application code paths to use the newer protobuf definitions introduced since the v1.3.9 release of OpenFGA or b) upgrade your gRPC clients to the newer protobuf definitions and make the upgrade to v1.4.0. If you want to be able to use the newer feature of Conditional Relationship Tuples, then we want developers to update their clients.

alee792 commented 11 months ago

I appreciate the prompt response! Breaking changes with gRPC are severe, but common, and I completely understand the desire to re-align your API. As long as we are given appropriate notification of breaking changes, we can do our best to accommodate, but were left a bit blind sided by this one.

Once a server has been upgraded to v1.4.0 or v1.3.9, is it possible to downgrade to v1.3.7? Ideally, we'd like to go back to pre-v1.3.3 to test a full migration path to test a forwards/backwards compatible client I've clobbered together. The release notes for v1.4.0 suggest that a downgrade will be compatible for all but conditional components of APIs, but I wanted to double check, given that there seem to be schema changes throughout several v1.3.* patches.

wilerson commented 8 months ago

We will soon be releasing some official documentation on the new Conditional Relationship Tuples feature and as part of that documentation we will have a more detailed overview of the impact of upgrades from v1.3.7 to v1.3.8 and up to v1.4.0.

Do we already have this?

a) stay on v1.3.7 and use a gRPC client that references the older protobuf definition(s) until you can upgrade client application code paths to use the newer protobuf definitions introduced since the v1.3.9 release of OpenFGA

Since the newer protobuf definitions are not compatible with OpenFGA v1.3.7, it'd be unfeasible to upgrade without causing a downtime. Any hints?

jon-whit commented 8 months ago

@wilerson the upgrade from v1.3.7 to >= v1.4.0, unfortunately, is a big upgrade with incompatibility on the gRPC client front. If you want a 0 downtime rolling deploy for the upgrade, then you could switch to temporarily use the HTTP API (I realize that is non-ideal, but it is an option). That's probably the quickest way to unblock the upgrade. Other ways could include to temporarily deploy two instances of OpenFGA with their own backing datastores and replicate between the two, but that would be a much larger effort.

Would the HTTP route work for your use case?

wilerson commented 8 months ago

Would the HTTP route work for your use case?

It'd be quite cumbersome, as we've setup a good chunk of our infrastructure around OpenFGA to use gRPC, and would need to rewrite a few wrappers around the client to use the HTTP API.