feat(cmd): Support Base64 encoded SSH_KEY

[felixbuenemann]

This adds a check to see if the value of SSH_KEY is already a Base64 encoded value, which is the case if "deis config:push" is used after using "deis config:pull" on an app that already has SSH_KEY set.

This fixes #274.

[felixbuenemann]

I tested the new behavior manually, but was unable to add automated tests, because the current location of the ssh key parsing code is poorly chosen and makes it hard to test.

In order to add sensible tests the code should probably be moved into a separate parseSshKey function which would be called from parseConfig instead of ConfigSet.

[codecov-io]

Codecov Report

Merging #309 into master will increase coverage by 0.21%. The diff coverage is 77.77%.

@@            Coverage Diff             @@
##           master     #309      +/-   ##
==========================================
+ Coverage   72.71%   72.93%   +0.21%     
==========================================
  Files          59       59              
  Lines        4109     4116       +7     
==========================================
+ Hits         2988     3002      +14     
+ Misses        793      785       -8     
- Partials      328      329       +1

Impacted Files	Coverage Δ
cmd/config.go	50.29% <77.77%> (+6.38%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2dddbe1...88397a1. Read the comment docs.

[Joshua-Anderson]

We have some public ssh key parsing logic in a package here, maybe add the private ssh key logic parsing here too?

Also, I might swap the logic, so you check if the key is base64'd before trying to read a file and parse it, which feels a little safer to me.

Thanks for contributing!

[felixbuenemann]

@Joshua-Anderson I've thought about this and I'm wondering wether this is the right approach.

The weird thing is that deis config lists all environment variables in the base64-decoded form, except SSH_KEY. If it was properly decoding SSH_KEY this workaround would not be needed at all.

[felixbuenemann]

OK, I've looked through the code and the reason that the SSH_KEY variable is base64 encoded is probably compatibility with existing heroku buildpacks. It's kinda strange, cause a private key is already in PEM format which also uses base64 encoding…

[felixbuenemann]

@Joshua-Anderson I've extracted the parsing code into a new parseSshKey function and added unit test for it as well as an integration style test in ConfigSet which ensures the code is called.

I've chosen to not put the new code into the ssh package, because it's implementation is too closely coupled to the specific behavior in ConfigSet to be re-used somewhere else, but I could move it if you disagree.

If you prefer I can squash to a single commit once the review is positive.

[felixbuenemann]

Oh and I also changed the SSH Private Key matching regex so it also matches ed25519 or other ssh keys converted to the new openssh key format as part of a refactor commit. I could move that into a single commit if it's a problem.

[felixbuenemann]

Fixed all errors from make test-style.

[felixbuenemann]

@Joshua-Anderson Stil waiting on review feedback from you.

[Joshua-Anderson]

Sorry about that. I was confused by the fact you check for ssh keys as well as ssh keys base64 encoded by deis, but I understand where you're coming from now. I'd like to manually test it, but code looks LGTM!

[mboersma]

I've tested this change locally and it works as advertised.