TritonDataCenter / sdc-docker

Docker Engine for Triton
Mozilla Public License 2.0
183 stars 49 forks source link

func sshGetMD5Fingerprint wasn't working when invoked 'sh', fixes #39. #40

Closed dwlf closed 9 years ago

dwlf commented 9 years ago

Not yet tested on recent version of OpenSSH.

rmustacc commented 9 years ago

This isn't an sh script though, why are we invoking it as sh? If we want it to be a proper sh script, there's a lot more that's wrong with it.

dwlf commented 9 years ago

That is an excellent question @rmustacc. @trentm thoughts?

I've confirmed that this change works when invoked with 'sh sdc-docker-setup.sh' on:

rmustacc commented 9 years ago

Sure, it works a bit when sh is actually bash. Unfortunately sh is not always bash (see, eg. smartos, debian, *BSD, etc.). An analysis as to why this change works is necessary and likely will need to be in a comment, as moving the assignment into the if in shell scripting is not generally done stylistically.

dwlf commented 9 years ago

My first choice would be to update docs to invoke with 'bash' and kill this pull request.

Other than the if hack, I haven't found another way for the the invalid option from ssh-keygen -E md5 -l -f "$sshPubKeyPath" 2> /dev/null not to exit the function and result in the empty assignment sshKeyId=.

I haven't explored trapping, but my strong preference will be to update docs to invoke with bash.

trentm commented 9 years ago

+1 to changing docs to say invoke with bash If we want to support with just vanilla sh, then that should be separate work (with test cases).