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 write command #242

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

Implement secrets write command #242

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

Comments

@tegefaulkes
Copy link
Contributor

tegefaulkes commented Jul 10, 2024

Specification

As part of the #32 issue refactoring, we need to implement secrets write command. This command is the most basic method of writing secrets.

secrets read

This command needs to read the contents of a secret in a vault. The contents will be outputted to stdout. Pretty much just the existing secrets get command. I don't think any changes need to be made to it.

The command takes the form of pk secrets read [secretPath]. The output can be redirected with pk secrets read [secretPath] > some file and piping into it shouldn't do anything. (see #242 (comment))

secrets write

This command needs to update or create the contents of a file. There will be two ways to provide the secret contents here. Coompared to current implementation, this pretty much is just secrets create but allows you to update a secret as well.

  1. Interactively via stdin. so if you call secrets write secretPath you can type in you contents to the terminal and send EOF (ctrl+d) to finish writing.
  2. Piping data in.

In both cases the behaviour should match cat > file.

Additional context

Tasks

  1. Implement secrets read command.
  2. Implement secret write command.
  3. create tests demonstrating behaviour for read.
  4. create tests demonstrating behaviour for write.
@tegefaulkes tegefaulkes added the development Standard development label Jul 10, 2024
Copy link

linear bot commented Jul 10, 2024

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Jul 10, 2024 via email

@tegefaulkes
Copy link
Contributor Author

I'm following the spec you wrote out where Imagine (IN ORDER OF PRIORITY): and read, write were top of the list. I can skip theses then since cat will basically cover that functionality anyway.

@CMCDragonkai
Copy link
Member

The read and write commands are not normal unix commands. The historical context for this is #32 (comment) (which should be added as the specific additional context above).

I would like cat to be an alias of the read command though as it's quite a common thing for shell users to use.

Copy link
Contributor Author

While technically using some shell shenanigans you can use cat to both read and write files. That functionality will not work too well for PK and the vaults. We will still use cat to cover the read case here. But we still need to create a write command that will write the stdin into the secret we want.

Copy link
Contributor

As discussed, the read case will be handled by the cat command, which has already been implemented into Polykey as of writing this. While write isn't standard UNIX commands, it would still need implementation.

We also wouldn't need to create a new handler for this. The edit command already has a handler which takes in a secret path, and updates the contents of the secret with the new, provided contents. However, it is a UnaryHandler. I can update that to be a ClientHandler, so it will accept streamed data instead of writing all the data in a big chunk.

@aryanjassal aryanjassal changed the title Implement secrets read and secrets write commands Implement secrets write command Sep 27, 2024
Copy link
Contributor

Technically, this issue is completed, as the linked PRs have correctly implemented the spec. However, we need to convert the UnaryHandler to RawHandler. I'm also not sure if we want local file system integration for this command.

Do I make another issue which continues this spec, converting to RawHandler and, if desired, implementing file system integration? Or the handler conversion issue is unrelated to this issue, and technically, the specification for this issue is done?

I need some clarification on this.

Copy link
Contributor Author

Using raw handlers is something we need to solve for everything at once. So we need to come up with a standard approach and apply it universally. Make a new issue for converting all the secret unix commands to using raw streaming for secret contents. Have it track everything that needs to be converted so a checklist for each command that needs to be updated. And also discuss the constraints involved for each one. For example some might just need to stream a single file such as this write command. Others needs the tar format streaming to handle a file tree or multiple files. Maybe we keep it standard across all and use the tar format for all of them? Its something that needs to be solved still.

Copy link
Contributor

That makes sense. I will make that issue and use it to track the discussion for converting all the commands to start using RawHandler.

Copy link
Contributor

MatrixAI/Polykey#822

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