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

Ssh key support #343

Merged
merged 3 commits into from
Feb 25, 2016
Merged

Ssh key support #343

merged 3 commits into from
Feb 25, 2016

Conversation

awlx
Copy link

@awlx awlx commented Feb 25, 2016

Adds the functionality to provide a ssh public and privatekey to use for SSH GIT connect.

ytti added a commit that referenced this pull request Feb 25, 2016
@ytti ytti merged commit 9ffa741 into ytti:master Feb 25, 2016
@danilopopeye
Copy link
Contributor

@awlx no tests for this? 😢

@awlx
Copy link
Author

awlx commented Feb 25, 2016

Sorry, was a quick fix to get it finally working. Tests will follow when I have time.

And tbh. First time I use ruby, no clue how this tests work ;).

@awlx awlx deleted the ssh_key_support branch February 25, 2016 17:37
@danilopopeye
Copy link
Contributor

@awlx I haven't test this, but it should be close 😉

-      it "will push to the remote repository using ssh" do
-        Oxidized.config.hooks.github_repo_hook.remote_repo = 'git@github.com:username/foo.git'
-        Rugged::Credentials::SshKeyFromAgent.expects(:new).with(username: 'git').returns(credentials)
-        gr.cfg = Oxidized.config.hooks.github_repo_hook
-        gr.run_hook(ctx).must_equal true
+      describe "will push to the remote repository using ssh" do
+        before do
+          Oxidized.config.hooks.github_repo_hook.remote_repo = 'git@github.com:username/foo.git'
+          Oxidized.config.hooks.github_repo_hook.pubkey = 'pubkey'
+          Oxidized.config.hooks.github_repo_hook.privkey = 'privkey'
+        end
+
+        it "using credentials" do
+          Rugged::Credentials::SshKeyFromAgent.expects(:new).with(username: 'git').returns(credentials)
+          gr.cfg = Oxidized.config.hooks.github_repo_hook
+          gr.run_hook(ctx).must_equal true
+        end
+
+        it "using public and private keys" do
+          File.expects(:expand_path).with('pubkey').returns('/pubkey')
+          File.expects(:expand_path).with('privkey').returns('/privkey')
+
+          Rugged::Credentials::SshKey.expects(:new).with(username: 'git', publickey: '/pubkey', privatekey: '/privkey').returns(credentials)
+          gr.cfg = Oxidized.config.hooks.github_repo_hook
+          gr.run_hook(ctx).must_equal true
+        end

@danilopopeye
Copy link
Contributor

@awlx another thing, what do you think of using the same keys that Rugged use: publickey and privatekey ?

/cc @ytti + @ElvinEfendi

@awlx
Copy link
Author

awlx commented Feb 25, 2016

Feel free, also thought about it. But I am lazy and so short config options are better ;).

@ytti
Copy link
Owner

ytti commented Feb 25, 2016

@danilopopeye publickey and privatekey are probably more descriptive yes.

@awlx awlx restored the ssh_key_support branch February 25, 2016 19:19
@awlx
Copy link
Author

awlx commented Feb 25, 2016

Fixed the variable names.

@danilopopeye
Copy link
Contributor

@awlx let me know if you have problem with the tests 😃

@awlx
Copy link
Author

awlx commented Feb 25, 2016

@danilopopeye how do I test if they work the way I expect them to ? :D

@danilopopeye
Copy link
Contributor

@awlx the other comment I've made, already does this.

it "using public and private keys" do
  # this line here
  Rugged::Credentials::SshKey.expects(:new).with(username: 'git', publickey: '/pubkey', privatekey: '/privkey').returns(credentials)
  gr.cfg = Oxidized.config.hooks.github_repo_hook
  gr.run_hook(ctx).must_equal true
end

The line I marked, tests the class Rugged::Credentials::SshKey must received a call to #new with the hash:
{ username: 'git', publickey: '/pubkey', privatekey: '/privkey' }.

Have I made sense?

@awlx
Copy link
Author

awlx commented Feb 25, 2016

Yeah, that makes sense. My question was more "how do I test the test?".

@danilopopeye
Copy link
Contributor

Oh, sorry! Must have needing more ☕! That diff is for the spec/githubhook_spec.rb file, it should be around line 115.

After that, just run rake test in the console.

@ElvinEfendi
Copy link
Contributor

@awlx another thing, what do you think of using the same keys that Rugged use: publickey and privatekey ?

👍 I also think we should keep it consistent with Rugged's keys.

@awlx
Copy link
Author

awlx commented Feb 26, 2016

It's already renamed and merged :). Tests will follow soon.

THX for the feedback!

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