Add exec command #19
Add exec command #19
Conversation
Codecov Report
@@ Coverage Diff @@
## master #19 +/- ##
==========================================
- Coverage 51.48% 49.63% -1.86%
==========================================
Files 8 8
Lines 606 683 +77
==========================================
+ Hits 312 339 +27
- Misses 254 306 +52
+ Partials 40 38 -2
Continue to review full report at Codecov.
|
box.go
Outdated
| @@ -150,6 +150,29 @@ func genkey(publicKeyFile string, privateKeyFile string) { | |||
| pemWrite(privateKey, privateKeyFile, "NACL PRIVATE KEY", 0600) | |||
| } | |||
|
|
|||
| func decryptEnvelopes(input string, decryptor DecryptionStrategy) (output string, err error) { | |||
There was a problem hiding this comment.
Please add test case where the base64 encoded envelope contains whitespaces and newlines, for example if it's broken into fixed-width blocks. I think there may be a stripWhitespace lacking somewhere in this function, like the decryptStream() and decryptEnvironment() commands do
commands.go
Outdated
|
|
||
| decryptedArgs[0] = path.Base(cmd) // By unix convention argv[0] has to be set to basename of command | ||
| for i, arg := range args[1:] { | ||
| decryptedArg, subErr := decryptEnvelopes(arg, crypto) |
There was a problem hiding this comment.
Could you consider refactoring atleast decryptStream() and possibly decryptEnvironment() to use the decryptEnvelopes() function as well? Cut's down the complexity now that you've extracted a nice reusable function here
main.go
Outdated
| }, | ||
| } | ||
|
|
||
| cmdExec.SetUsageTemplate(`Usage:{{if .Runnable}} |
There was a problem hiding this comment.
Why is this needed? None of the other commands need to specify a convoluted template like this?
There was a problem hiding this comment.
Added a comment explaining the rationale
|
|
||
| # Decrypt secrets | ||
| if [ "$SERVICE_PUBLIC_KEY" != "" ]; then | ||
| SECRETS=$(secretary decrypt -e --service-key=/service/keys/service-private-key.pem) |
There was a problem hiding this comment.
The exec command example doesn't support using the service key. This optional key provides more security by ensuring that the secret can only be decrypted on a specific set of machines that hold the private service key on local disk. Not sure how to solve this right off, could we have a chat about it?
There was a problem hiding this comment.
Yes, let's discuss this briefly tomorrow. Could it be solved by allowing for either --service-key or SERVICE_PUBLIC_KEY environment variable?
There was a problem hiding this comment.
Hmm... it seems there is support already here: https://github.com/meltwater/secretary/pull/19/files#diff-7ddfb3e035b42cd70649cc33393fe32cR162 (line 162). However, we do remove the logic of "by default use /service/keys/service-private-key.pem if it exists".
There was a problem hiding this comment.
Now supports setting a path to a pem file in SERVICE_PRIVATE_KEY
Also updated documentation about how to properly set this in marathon config
|
@dezmodue or other @meltwater/foundation members. Can we merge this and release a new version of secretary? It would enable us to more easily use non-meltwater base images that don't necessarily have a shell. |
When embedding standalone static binaries in a docker container, it feels nice to use the empty "scratch" docker image as a starting point. However, this fails when also wanting secretary support, since the current workflow requires a shell.
This PR adds the support for
secretary exec -- cmd [args..]which will decrypt all environment variables and all command line arguments without requiring an embedding shell to do the work.