Proposal: gRPC Refactoring #1881
asher-pem-arm
started this conversation in
General
Replies: 0 comments
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Uh oh!
There was an error while loading. Please reload this page.
-
To facilitate future extensions to Labgrid with functionality such as authentication/authorization as well as to improve the experience integrating Labgrid with external systems, we propose as suite of backwards compatible extensions and refactoring to the gRPC API.
The proposal includes the following groups of changes:
StartupDonemessage (used in theClientStreamandExporterStream) and metadata-based identity passingThe current API relies on the
ClientStreambeing created and theStartupDonemessage being sent before the client is able to successfully issue some other RPCs. The requirement to setup and maintain theClientStreamadds complexity to 3rd party clients and integrations that wish to interact with the Labgrid API.We propose the deprecation of the
StartupDonemessage and a replacement mechanism, gRPC metadata, for transmitting the client identity and version. The changes will ensure backwards compatibility for older clients that still send their identity over theClientStream.Corresponding changes are proposed for the
ExporterStream.Certain functionality, like listing the Resources on the Coordinator is only available by processing the updates provided over the
ClientStream. We propose adding explicit unary RPCs to achieve this functionality. This should simplify thelabgrid-clientcodebase and allow for easier integration with 3rd party clients.We propose the standardisation of the existing unary RPCs to improve naming and functionality (e.g. pagination). We have based our proposals, partly, on Google's AIPs.
Deprecation of the
StartupDonemessage (used in theClientStreamandExporterStream)Overview
The successful transmission of the
StartupDonemessage over theClientStreamis a prerequisite for a number of other RPCs to succeed (AddPlace,AcquirePlace,AllowPlace,CreateReservation).Proposal
We propose replacing the
StartupDonemessage with three gRPC metadata KV pairs that are transmitted on each RPC call:x-lg-usernamelabgrid-client. This KV pair is not relevant tolabgrid-exporter.x-lg-hostnamelabgrid-clientorlabgrid-exporteris being used from.x-lg-user-agentlabgrid-clientor a 3rd party client or integration) that is making the RPC call. This key MAY be omitted when the API is called by an entity other thanlabgrid-client.Implementation
The
labgrid-coordinatorimplementation MUST retain the ability to infer user identity from theStartupDonemessage to be backwards compatible with olderlabgrid-clientversions.The
labgrid-clientandlabgrid-exporterno longer send theStartupDonemessage and instead uses the metadata KV pairs described above.This change will remove the need for a system that directly consumes the gRPC API to establish a
ClientStreambefore calling the unary RPCs. This reduces the amount of state-management logic needed on clients, making integration with Labgrid easier.Creation of new unary RPCs
Overview
We propose transitioning the API to use unary RPCs where possible, in a backwards compatible way. Unary RPCs have the benefit that they are easier to use within client code compared to stream RPCs due to less state-handling being needed. The new unary RPCs are more tightly scoped to make the future implementation of authentication/authorization functionality easier. This change is facilitated by the deprecation of the
StartupDonemessage.Proposal
The following new RPCs are proposed:
GetPlacePlace.Places. This could be costly for larger deployments of Labgrid with many places perlabgrid-coordinator. It is simpler for a client interested in a singlePlaceto query the information about thatPlacedirectly, rather than listing allPlaces and having to filter for the desiredPlaceon the client side.ListResourcesResources.Resourcesby establishing aClientStreamand populating a local cache based on thelabgrid-coordinator-sentClientOutMessages. This RPC simplifies this process for clients that do not need real-time updates and would like to easily list allResources on thelabgrid-coordinator.ListPlaceResourcesResources associated with aPlace.Resourcesassociated with a place by establishing aClientStreamand subscribing for all resource updates. This RPC simplifies this process for clients that do not need real-time updates and would like to easily list allResources for an acquired place on thelabgrid-coordinator.UnsharePlacePlacethat they previously had access to via a share/allow.AllowPlace(which will be superseded bySharePlace, described below). This RPC adds the ability to remove a user that was previously added withAllowPlace/SharePlace.Standardisation of the existing unary RPCs
Overview
We propose a combination of replacing RPCs and the addition of renamed fields in existing messages to create a more uniform API. For backwards compatibility purposes, the existing RPCs will be deprecated but not removed.
The naming of RPCs, message fields and the implementation of functionality such as pagination is influenced by Google's AIPs.
Proposal
Creation of new RPCs to replace existing RPCs
Existing RPC to deprecate
New RPC
Proposed definition
Functional differences to existing RPC
Placein the RPC responsehttps://google.aip.dev/158
Clarifications of RPC
messagefield names and typesWe believe some of the existing field names have ambiguous names which could cause confusion. Additionally, the handling of timestamps can be improved.
As renaming of fields in Protobuf messages causes changes to the JSON serialisation and generated stubs, we cannot make changes to the field names without potentially affecting other direct users of the gRPC API. Instead, we suggest adding new fields with improved names and deprecating the old fields. The implementation will need to continue to support the old fields to maintain backwards compatibility with older clients.
Protobuf Message
Existing Schema
New Schema
Rationale
Reservationtokencan be misunderstood to mean something that relates to authentication/authorization functionality.The Reservation
tokenacts as an identifier for the Reservation, so we propose making this unambiguous by changing it toid.PlaceBeta Was this translation helpful? Give feedback.
All reactions