Add command to encrypt all files if no argument is provided #61
Conversation
add: encrypt all files if no argument is provided. Add safety checks for existence of crypted and non-crypted files on disk decrypt: add safety checks for existence of crypted files on disk. Add log messages. exec: defer file handler close
heavenly999
changed the title
heavenly999
requested review from
onyxraven,
elementalvoid,
ibottamike and
physik932
as code owners
|
Hey, just wanted to reply back that I've seen the PR and will review it closer later in the week :) |
| Args: cobra.MinimumNArgs(1), | ||
| Aliases: []string{"a", "e", "add", "encrypt"}, | ||
| Use: "add [files ...], add", | ||
| Short: "add file to the encryption list and encrypt file. No argument encrypts all in sops config.", |
There was a problem hiding this comment.
Hey so I'm thinking that the proposed reencrypt is really update.
Add has the semantics of always adding a new entry to the list.
Update can then be a clean 'already exists (added) and update encrypted file from disk'. Then the 'empty args' version of update could be the proposed.
IMO, this feels cleaner than adding a 'force' flag, which incurs all the logic changes below.
There was a problem hiding this comment.
update would have:
no-clean for not cleaning up plaintexts
recursive, r, to update recursive from the given path
arg is a list of files, or directories (where all tracked in the directory are updated, if plaintext exists), default .
| // Check if encrypted file already exists on disk | ||
| if _, err := os.Stat(encryptedFilePath); !os.IsNotExist(err) && !forceOverwrite { | ||
| fmt.Printf("Encrypted version of file %s already exists on disk. Do you want to overwrite it? [y/N]: ", encryptedFilePath) | ||
| var input string |
There was a problem hiding this comment.
we don't do interactive inputs anywhere else in sopstool. I'd like to avoid it I think.
| } | ||
|
|
||
| if !noClean { | ||
| err := encrypter.RemoveFile(fn) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| fmt.Println("Cleaned up plaintext for:", fn) |
There was a problem hiding this comment.
can we maybe do some of the extended output in this at a verbose level?
| for _, f := range filesToDecrypt { | ||
| cryptfile := fileutil.NormalizeToSopsFile(f) | ||
| if _, err := os.Stat(cryptfile); os.IsNotExist(err) { | ||
| fmt.Println("Encrypted version of file does not exist:", f) |
There was a problem hiding this comment.
a verbose level for this may be good, and maybe should be associated with the allowFail flag.
| @@ -46,6 +46,8 @@ func (ew execWrap) RunCommandStdoutToFile(outfileName string, command []string) | |||
| if err != nil { | |||
| return err | |||
| } | |||
| defer outfile.Close() // Close the file after cmd completes | |||
There was a problem hiding this comment.
in this case we are explicitly running Close() below, because we want it to be in the order of Close, Remove in the case of an error. (defer are otherwise run in LIFO stack order, so the remove would happen before the close)
|
|
||
| func init() { | ||
| RootCmd.AddCommand(addCmd) | ||
|
|
||
| addCmd.Flags().BoolVarP(&noEncrypt, "no-encrypt", "n", false, "Do not encrypt the file after adding") | ||
| addCmd.Flags().BoolVar(&noClean, "no-clean", false, "Do not clean up plaintext after encrypting") | ||
| addCmd.Flags().BoolVarP(&forceOverwrite, "force", "f", false, "Force overwriting of encrypted files if they already exist") |
There was a problem hiding this comment.
a couple proposals to consider:
all,Awhich will, if given a list, directory, or empty arg (synonym.), will add any missing files. This is analogous togit add -Aignore,iwhich will ignore any already added files (adding only new ones)
I'm partial to A, especially with the path arg.
add:
recursive,r, to add recursive from the given path
| Args: cobra.MinimumNArgs(1), | ||
| Aliases: []string{"a", "e", "add", "encrypt"}, | ||
| Use: "add [files ...], add", | ||
| Short: "add file to the encryption list and encrypt file. No argument encrypts all in sops config.", |
There was a problem hiding this comment.
update would have:
no-clean for not cleaning up plaintexts
recursive, r, to update recursive from the given path
arg is a list of files, or directories (where all tracked in the directory are updated, if plaintext exists), default .
Story/Issue Link
Issue #60
Background
addcommand, we will ensure it encrypts all files when no argument is provided and will add checks to determine the existence of both encrypted and non-encrypted files on disk.decryptcommand will receive safety checks to confirm the presence of encrypted files on the disk, coupled with additional log messages to keep users informed.execcommand, we'll implement a deferred file handler close to optimize resource management.