Updating behaviour of VaultsSecretsGet to align with requirements of secrets cat

[aryanjassal]

Description

The current VaultsSecretsGet only gets a single secret from the vault, and returns it in the form of a UnaryHandler, meaning for large files, the RPC call will timeout. It also fetches only one file at a time, so behaviour like UNIX's cat command isn't possible.

This PR aims to fix that issue by switching over the RPC handler to a ServerHandler, supporting larger files. This PR also adds support for listing the contents of multiple files in order, like what cat does.

Issues Fixed

• Relates to Implement secrets cat command Polykey-CLI#243
• Closes ENG-356 (link)

Tasks

• 1. Switch UnaryHandler to StreamHandler
• 2. Implement support for displaying contents of multiple files
• 3. Implement support for listing multiple files across multiple vaults
• 4. Update old test and add new tests

Final checklist

• Domain specific tests
• Full tests
• Updated inline-comment documentation
• Lint fixed
• Squash and rebased
• Sanity check the final build

[linear]

ENG-356 Implement `secrets cat` command

[aryanjassal]

I have implemented basic functionality for displaying file contents for multiple files, and updated the tests in tests/client/handlers/vaults.test.ts.

I still need to review the issue spec and all the code to ensure everything's ready for review.

[aryanjassal]

MatrixAI/Polykey-CLI#243 (comment)

I'm going to go ahead with the assumption that we need to implement support for listing contents for files from multiple vaults concurrently, as that makes sense and retains consistency with UNIX commands and secrets rm.

I will wait for review of my approach from secrets rm, and if my implementation seems correct, I will implement that in secrets cat too.

[aryanjassal]

The naming of secret commands has changed from what it was. The intention was to keep using the pattern of SecretsGet, SecretsPut, SecretsDel, but to better align with the new commands, these names have been changed to SecretsGet (I haven't changed this name yet), SecretsEdit, SecretsRemove.

We are kinda moving away from the pattern. This was hard to detect as this change was made one-at-a-time across multiple PRs. We might need to look into this and set a standard pattern to follow for ensuring consistency.

[tegefaulkes]

Small changes, just check your tests are only testing the specific condition we want.

[aryanjassal]

While I am writing the tests, I can see that we use RPC calls everywhere, even when we don't need to. For example, if we are testing vaultsSecretsGet and vaultsSecretsRemove, then we will have RPC handlers for vaultsSecretsNew, vaultsSecretsMkdir, vaultsSecretsStat, etc. as other RPC handlers we use to help perform vault operations in the tests.

This introduces slowdowns as each RPC call takes time to complete. A faster method of doing the same would be to acquire a vault, and perform writeF on the vault filesystem directly. It should speed up all our tests.

@tegefaulkes and I had a chat about this, and he was also of the same opinion as I, but he mentioned how all the tests would need to be updated to ensure consistency.

How should I deal with this? Do I make a new issue tracking this? It should be relatively straightforward, so maybe this can be something @brynblack could attempt once she gets into Polykey Core development.

[tegefaulkes]

It's some thing we can address later. I think we should make a new issue for it. It should be a general review and refactor of the vaults domain tests. I think one for the CLI and one for Polykey. You mentioned that the vaults tests is one big test file. We should look into splitting that out again but we need to check if it still crashes the CI. The other part is dealing with the vaultOps.test.ts testfile. Currently its the longest running tests by far so if we can speed it up or split it into parts that would help as well.

[tegefaulkes]

Looks good to me.

[aryanjassal]

All tasks have been completed for this PR, all checks are passing, and reviews has also been approved. Merging.