dss icon indicating copy to clipboard operation
dss copied to clipboard

Remove gRPC layer

Open barroco opened this issue 3 years ago • 2 comments

The current server exposes a gRPC interface, which handles HTTP requests using a separate HTTP gateway. Exposing an HTTP interface directly will substantially reduce debugging effort and improve insight into errors while reducing system complexity. We already have a tool that produces a complete HTTP server with handler stubs directly from an OpenAPI YAML; this task is to use and expand that tool to entirely replace our gRPC server infrastructure. An important aspect of this change is to update the deployment Jsonnet files and verifying the functionality.

Rationale and design notes: https://github.com/interuss/dss/wiki/Update-DSS-webserver-architecture

  • [x] Phase 1: openapi-to-go-server
  • [ ] Phase 2: Migrate DSS webserver
  • [ ] Phase 3: Update the deployment Jsonnet files and verify the functionality.

References

  • Open API to go server tool: https://github.com/interuss/dss/tree/master/interfaces/openapi-to-go-server
  • Integration tests: https://github.com/interuss/dss/tree/master/monitoring/prober

barroco avatar Oct 13 '22 13:10 barroco

Here follows a tentative plan that also gives some insight on the approach I'm taking and points where I might need clarifications.

The development is done on this branch of the orbitalize/dss repo and you can track the diff through this page.

Feedback greatly appreciated!

Plan

  • [x] Generate interfaces and models from OpenAPI definitions
    • [x] Generate go source files in:
      • pkg/api/rid_v1
      • pkg/api/rid_v2
      • pkg/api/scd_v1
    • [x] Integrate generation in Makefile
  • [x] Replace handlers For each API, there is a Server struct that currently implements the gRPC server interface. Each of those handlers is to be replaced by its REST counterpart generated by the tool.
  • [x] Replace models conversion
  • [x] Create aux API Create OpenAPI definition of the API and follow the same previous steps as the other APIs.
  • [x] Replace all middlewares
    • [x] Error handling The error handling is quite complex at the moment because of the architecture: this middleware intercepts errors from gRPC handlers to log them and replace them with a specially-formatted error for the gateway. Then the gateway performs a mapping of gRPC status to HTTP status with complex special cases. The idea here is to greatly simplify this handling by:
      • removing altogether the error handling middleware
      • let the handlers generate the appropriate final HTTP error
      • they do so by using an helper function that functionally replaces the middleware:
      // Handle parses and handles an error that happen in a REST handler. The error
      // is logged and a message appropriate for the requesting client is returned.
      func Handle(ctx context.Context, err error) *string { ... }
      
    • [x] Logging Both the gateway (dump) and the core-service (dump and normal) have logging middlewares, there are a total of 3. Some of them are optional (through CLI flag) to allow dumping the raw requests. The idea is to merge all those in a single logging middleware that takes a boolean parameter to control the dump of raw request in the logs.
    • [x] Authorization The idea is to adapt the existing middleware to fit the interface expected by the generated REST server files.
    • [X] Validation The existing validation middleware only checks if the UUID are correctly formatted. This check is actually performed a second time when the UUIDs are parsed in the handlers. This middleware will simply be removed, if there is additional validation to be made in the future it will be made from within the handlers.
  • [x] Merge the main() functions From core-service and http-gateway.
  • [x] Update deployment and CI
  • [x] Update READMEs and documentation

Open points

  • [x] There is a aux API: what to do with it? Create an OpenAPI definition and generate the files as for the others?

mickmis avatar Oct 18 '22 08:10 mickmis

@mickmis (cc: @BenjaminPelletier) As discussed, I created a feature branch from master for this issue: features/822-remove-grpc-layer

barroco avatar Oct 19 '22 20:10 barroco