Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement secrets cat command #243

Closed
tegefaulkes opened this issue Jul 10, 2024 · 9 comments
Closed

Implement secrets cat command #243

tegefaulkes opened this issue Jul 10, 2024 · 9 comments
Assignees
Labels
development Standard development

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Jul 10, 2024

Specification

As per the #32 issue refactor, we need to implement a simplified cat command as secrets cat.

This will function identically to cat while omitting some of the options for now. It pretty much combines the functionality of read and write commands along with the concatination cat is known for.

Uses are

  1. read the contents of a secret secrets cat secretPath or file secrets cat filePath.
  2. write the stdin to a secret cat > secretPath or file cat > filePath. stdin will support pipes as well.
  3. concatenating files together in the output. secrets cat secretPath filePath secretPath/*.

Pretty much the the functionality boils down to two things

  1. Will output the contents of all specified files to stdout in the order they are specified.
  2. If no files are specified then it will just output stdin to stdout.

Redirection into a secret path might not be possible with how commander parses inputs and how shell works, at least, not with the cat > secretPath syntax. We'll need to prototype that. If it doesn't work, we'd have to use an option --redirect secretPath to enable the functionality. (see comment)

Additional context

Tasks

  1. Create a secrets cat command
  2. If no arguments are provided it will take stdin and output to stdout.
  3. If arguments are provided it will write their contents to stdout in the order they are provided.
  4. Support specifying normal files along side secret paths.
  5. Support specifying wildcards and possibly globbing for the paths.
  6. Support redirecton into a secret using secretPath syntax. Failing that provide an option that handles that behaviour. (see comment)
@tegefaulkes tegefaulkes added the development Standard development label Jul 10, 2024
Copy link

linear bot commented Jul 10, 2024

Copy link
Contributor Author

@aryanj you want to work on this one next after completing your current work.

Copy link
Contributor Author

For streamlining the basic functionality. just focus on creating the cat command and having it handle multiple paths within a single vault. Don't worry about using the seralizer utility for now. I'm starting to think I need to review how it works and it's use based on what we've learned.

Focus on tasks 1 and 3. Use a ServerHandler to stream the contents as ContentMessage much like the commands are already doing.

In the future we'll have to update all the commands with the serialiser optimisation. And the other tasks for this issue will be a future PR.

Copy link
Contributor

Focus on tasks 1 and 3. Use a ServerHandler to stream the contents as ContentMessage much like the commands are already doing.

The current VaultsSecretsGet is a UnaryHandler. Do I change that to a ServerHandler? Doing it so would be incompatible with MatrixAI/Polykey#799 as that PR implements RawHandlers for VaultsSecretsGet and VaultsSecretsSet. Should I make a new PR for converting VaultsSecretsGet to a ServerHandler?

@tegefaulkes
Copy link
Contributor Author

Create a separate PR for this and keep it simple. I'll need to have a look at MatrixAI/Polykey#799 and be more hands on with it later. For now we're just focusing on the simple implementations of the commands so we can build them up incrementally.

Copy link
Contributor

It is not clear in the issue spec, but do we need to support concatenating files across multiple vaults? This comment requires this behaviour for secrets rm, to allow the command to delete secrets across vaults concurrently.

The spec for secrets rm had this as a requirement. I'd assume this should also be implemented for secrets cat and any other UNIX commands which takes in a variadic number of arguments to ensure consistency.

Copy link
Contributor

aryanjassal commented Sep 16, 2024

The purpose of the cat command is to concatenate files together. Alternatively, it has been used as a way to quickly read file or view its contents, but that wasn't the original intent of the command.

The redirect operator > is dependent on each terminal's implementation. As it is handled by the terminal, the syntax polykey secrets cat vault:secret1 vault:secret2 > vault:secret3 is not possible. An idea (which has been mentioned in the issue spec) could be adding an option like --redirect but that would add unnecessary complexity to the command.

Instead, another command will be written to take stdin and write that to a specified secret instead. We would need to write up the spec for a command like polykey secrets write, and the redirection could be discussed/implemented as a part of it.

For this issue, redirection to another secret isn't part of the spec anymore.

@CMCDragonkai
Copy link
Member

Demo please? Where's the gif.

Copy link
Contributor

I forgot to upload the gif yesterday. Here it is: #282 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
Development

No branches or pull requests

3 participants