MatrixAI / Polykey

Polykey Core Library
https://polykey.com
GNU General Public License v3.0
31 stars 4 forks source link

Migrate all `VaultsSecrets` handlers to `RawHandler` #822

Open aryanjassal opened 1 month ago

aryanjassal commented 1 month ago

Specification

Most, if not all, VaultsSecrets handlers transfer binary data. Doing so using plain JSON isn't the most efficient, as the required bandwidth can be as high as 200% of actual data size, making regular JSON extremely inefficient for transferring binary data.

This is solved by the RawHandler, which is able to stream raw bytes of data directly, bypassing the large overhead. However, it also has many issues, like not supporting error serialisation by default. As such, any errors thrown within the context will hit the transport layer, causing a read 0 error.

To solve this, each handler needs to implement its own method of error serialisation and deserialisation. The handler side needs to serialise the errors so they are passed through as binary data, and the client side needs to deserialise the data back into the original error. Serialising and deserialising all possible errors is not feasible, and should not be done either. Only create serialisation and deserialisation handlers for relevant errors that are expected, and raise a generic error for all other, unexpected errors.

Additional context

Tasks

  1. Migrate the following to use RawHandlers
    • [ ] VaultsSecretsEnv (check note 1)
    • [ ] VaultsSecretsGet
    • [ ] VaultsSecretsMkdir
    • [ ] VaultsSecretsNew
    • [ ] VaultsSecretsNewDir (check note 2)
    • [ ] VaultsSecretsRemove
    • [ ] VaultsSecretsRename (check note 3)
    • [ ] VaultsSecretsWrite

Notes

  1. The VaultsSecretsEnv is an interesting case. It basically performs the same as VaultsSecretsList. The source code was actually copied from the old list handler, but was never updated. Requirements need to be clarified and the command needs to be updated before we can figure out if we need to convert this handler too, or if another handler suits it better.
  2. The VaultsSecretsNewDir will be rendered obsolete with the introduction of VaultsSecretsCopy and VaultsSecretsRename. This list will be updated then, but for the time being, it is reflecting status quo.
  3. This command will be changed to act as the mv command on unix. Then, it will need to be a RawHandler.
  4. The following VaultsSecrets handlers were excluded for the given reasons:
    • VaultsSecretsStat: The data it returns can be very nicely serialised into JSON. So, instead of converting command to use RawHandler, we can simply convert it to DuplexHandler to handle performing stat on multiple files instead of doing it on a single file with UnaryHandler.
    • The VaultsSecretsList handler currently sends over only the file path and the file path. This is easily serialisable by JSON without significant performance/bandwidth issues. Even in the future, we would only need to pass in the stat alongside the output. This shouldn't require the use of RawHandler, so it will be omitted from the conversion unless a reason comes up.
linear[bot] commented 1 month ago

ENG-424 Migrate all `VaultsSecrets` handlers to `RawHandler`

CMCDragonkai commented 1 month ago

This needs to be weighed up against making the RPC too unique to our usecases and making it more difficult to use in other clients.