Skip to content
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

Adding passwordless SSH and fixing some of the spec files #141

Merged
merged 8 commits into from
Jul 17, 2015
Merged

Adding passwordless SSH and fixing some of the spec files #141

merged 8 commits into from
Jul 17, 2015

Conversation

miroswan
Copy link

  • Adding passwordless SSH
  • Fixing issue with legacy include statements in default spec tests.

@coderanger
Copy link
Contributor

Love the idea, but we can't do this with a static private key :-/ The security issues would be too dangerous. Any chance you could change this to generate a new key pair using something like https://stackoverflow.com/questions/5270386/generate-ssh-keypairs-private-public-without-ssh-keygen ?

@miroswan
Copy link
Author

I was actually thinking about generating the key on the host and injecting the public key into the container. I think ssh-keygen would have to be installed on the host, which might be problematic for Windows users. Perhaps there is another way. I'll take a look at that stackoverflow link.

@coderanger
Copy link
Contributor

Yeah, you can do it all in Ruby code using the ssh_type and to_blob methods mentioned in that post.

@coderanger
Copy link
Contributor

Also if you want to be really fancy, you could use something like https://github.com/coderanger/kitchen-sync/blob/master/lib/kitchen-sync/rsync.rb#L46 to grab an existing identity from ssh-agent and then fall back to creating a new one if that fails.

@miroswan
Copy link
Author

I have some new code to start. I'm using the sshkey library to generate the pairs. Let me know what you think. I'm assuming it could use some graceful error handling. At least it's a working draft.

@coderanger
Copy link
Contributor

Given this can be done easily with net-ssh already, I'm not sure it is worth the extra dependency.

@miroswan
Copy link
Author

Kinda like how easy sshkey makes it, but I can take a look at net-ssh soonish.

@miroswan
Copy link
Author

Using net/ssh now.

@jeffbyrnes
Copy link

👍 to this, I’ve tested and it fixes #110 and #81 as far as I can tell.

@coderanger
Copy link
Contributor

Love it. @portertech any thoughts on overall thumbs up/down? If not I'll merge this soon.

@@ -126,6 +131,22 @@ def docker_command(cmd, options={})
run_command("#{docker} #{cmd}", options.merge(:quiet => !logger.debug?))
end

def generate_keys
if ! File.exist?(config[:public_key]) or ! File.exist?(config[:private_key])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruby style? Maybe this should be !File.exist?()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or use unless 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, also needs || instead of or but I can fix that up post merge.

@miroswan
Copy link
Author

@coderanger Did you want me to make these adjustments? I don't mind.

@miroswan
Copy link
Author

I updated. I chose not to go with unless because it's sometimes unclear if the negation applies to both. Did correct the style though.

@portertech
Copy link
Contributor

299

portertech added a commit that referenced this pull request Jul 17, 2015
Adding passwordless SSH and fixing some of the spec files
@portertech portertech merged commit 2f1dda3 into test-kitchen:master Jul 17, 2015
@portertech
Copy link
Contributor

@miroswan Thank you 👍 Will try to get this into a release before the weekend!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants