-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
Add support for encrypted environment variables #909
Conversation
Signed-off-by: David Challoner <dchalloner@gmail.com>
Signed-off-by: David Challoner <dchalloner@gmail.com>
👍 Would be so useful!! |
Signed-off-by: David Challoner <dec@zestfinance.com>
If I understand correctly, the use case here is being able to store encrypted ENV-vars in your docker-compose.yml, and have compose decrypt it before using (if you pass the decryption key / secret)? Personally I'm -1 on this; IMO it gives the impression that those values are "safe" (because they are encrypted), whereas they still get sent to the daemon (and added to the container) unencrypted. If you want to keep out your "secrets" from your docker-compose.yml, compose offers other ways to do this. You can set the variables before running I can see the convenience of this PR, but I think that encrypting/decrypting values in a docker-compose.yml could also be handled by an external application / helper tool that decrypts the values before passing it to compose in some way. Just my 0.02c, I'm not a maintainer. |
@thaJeztah I get your point but |
I fully understand that just about any system will need secrets, but keeping them in source control (even encrypted), just isn't the way to handle that. Handling secrets for deployment is not something new, and various solutions for that exist. You can Google for "keep passwords out of source control", here's an example; Handling secret credentials in Ruby on Rails Besides, if you use an env-file and keep that out of source control, you only have to copy that file to your project directory before starting/deploying and compose will handle this automatically, without external tools. |
I understand there are different opinions on this but you need to keep your credentials somewhere. If not in version control then some other repository (i.e lastpass). This uses AES-256 with a fairly ridiculous number of rounds of PBKDF2 plus a salt so it's probably more secure than most roll-your-own solutions and eliminates the need to use a separate service. You got to keep things simple and low overhead otherwise people will do dumb things like use email or god forbid postit notes to pass credentials around. |
This looks like a useful PR, but I'd really like to see this get introduced on top of #845, instead of introducing additional complexity that we'll have to accommodate in order to provide a top-requested feature from the community. It shouldn't be too hard to add this logic into the new config module that that PR introduces. |
I'm -1 on this. It introduces a lot of complexity to serve a use case that, so far, no-one has asked for. I don't want to start encrypting anything on the user's behalf unless there's a really compelling argument for it. |
I only particularly care that it not be introduced before environment variable support. I agree with the -1ers about the complexity it introduces. |
-1 Any "encrypted" value added this way would be exposed to everyone through |
@dnephin also true, I hadn't considered that. |
I think the main use-case is that you keep the fig.yml in git/vcs without storing plain-text passwords in it so it doesn’t matter that they are exposed via |
Fig and docker both support environment variables as only keys, and they both take the value from the environment, so I don't think you'd need to modify fig.yml. Just add the variable names to the environment list, and set the environment variables before running fig. |
They also both support environment variable files (docker: |
Oh, sorry - wasn’t aware of that! In that case I can solve this in another way - thanks! |
You can find links to the documentation for them in my earlier comment: #909 (comment). Hope you get it working for your situation! |
Thanks..must have overseen that. Going through other issues/PRs I thought env_file was not merged yet. |
Can you please sign your commits following these rules: $ git clone -b "master" git@github.com:Katlean/fig.git somewhere
$ cd somewhere
$ git rebase -i HEAD~6
editor opens
change each 'pick' to 'edit'
save the file and quit
$ git commit --amend -s --no-edit
$ git rebase --continue # and repeat the amend for each commit
$ git push -f This will update the existing PR, so you do not need to open a new one. |
Resolves #908 which I just opened.
Seemed simple enough to implement for our purposes though I understand if it can't be merged due to the added dependency etc.