[secrets] Add secrets download and upload #1720
Conversation
internal/boxcli/secrets.go
Outdated
| @@ -16,6 +16,19 @@ type secretsFlags struct { | |||
| config configFlags | |||
| } | |||
|
|
|||
| func (f *secretsFlags) genSecrets(cmd *cobra.Command) (*envsec.Envsec, error) { | |||
There was a problem hiding this comment.
genSecrets: are we generating them? :)
There was a problem hiding this comment.
Thoughts on Envsec() to indicate the struct being returned?
internal/boxcli/secrets.go
Outdated
| command := &cobra.Command{ | ||
| Use: "download <file1>", | ||
| Short: "Download environment variables into the specified file", | ||
| Long: "Download environment variables stored into the specified file (most commonly a .env file). The format of the file is one NAME=VALUE per line.", |
There was a problem hiding this comment.
could we add something about unless --format is specified?
internal/boxcli/secrets.go
Outdated
| command := &cobra.Command{ | ||
| Use: "upload <file1> [<fileN>]...", | ||
| Short: "Upload variables defined in a .env file", | ||
| Long: "Upload variables defined in one or more .env files. The files " + |
There was a problem hiding this comment.
again, need to modify it to mention --format or json
internal/devbox/secrets.go
Outdated
| WorkingDir: d.ProjectDir(), | ||
| EnvID: envsec.EnvID{ | ||
| EnvName: d.environment, |
There was a problem hiding this comment.
is it possible for d.environment to not be set? If so, could we add a validation check in this function?
There was a problem hiding this comment.
it's already validated in devbox.Open
internal/devbox/secrets.go
Outdated
| }, nil | ||
| } | ||
| envsecInstance.EnvID.ProjectID = project.ProjectID.String() | ||
| envsecInstance.EnvID.OrgID = project.OrgID.String() |
There was a problem hiding this comment.
its okay for now, but I feel the API could be made a bit safer by initializing EnvID as a complete struct in one place...
having it split between UninitializedSecrets and here feels like it could lead to bugs over time (once someone less experienced with this code refactors it, etc.)
There was a problem hiding this comment.
yeah, this weirdness is to fix a bug that secrets init would not work because it depended on the project already existing. Agree it's a bit awkward API.
2 possible fixes:
- We can always set the full EnvID object. Re-writing is fine.
- The
UninitializedSecretsdoesn't really need EnvID set. It just felt weird not to set it.
Summary
Adds
Also, updated envsec dependency that has a simpler API.
How was it tested?