You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by psiniemi <no...@github.com> on 2014/03/11 15:36:41 UTC
[jclouds] Add ssh-agent support via jsch agentproxy (#312)
ssh-agent is a more secure way of providing private ssh keys to ssh connections since an attacker would have to have access to the memory of the client to get access to the unencrypted key. Keys without encryption are accessible on the disk.
You can merge this Pull Request by running:
git pull https://github.com/NitorCreations/jclouds master
Or you can view, comment on it, or merge it online at:
https://github.com/jclouds/jclouds/pull/312
-- Commit Summary --
* Add ssh-agent support via jsch agentproxy
-- File Changes --
M drivers/jsch/pom.xml (10)
M drivers/jsch/src/main/java/org/jclouds/ssh/jsch/JschSshClient.java (35)
M drivers/jsch/src/main/java/org/jclouds/ssh/jsch/SessionConnection.java (15)
M project/pom.xml (17)
-- Patch Links --
https://github.com/jclouds/jclouds/pull/312.patch
https://github.com/jclouds/jclouds/pull/312.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for reviewing @demobox !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38330728
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> OpenSSHKeyFile key = new OpenSSHKeyFile();
> key.init(loginCredentials.getPrivateKey(), null);
> ssh.authPublickey(loginCredentials.getUser(), key);
> + } else {
> + AgentProxy proxy= SshjSshClient.getSSHAgentProxy();
[minor] Space before `=`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856700
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #679 UNSTABLE
Not sure if these are [real test failures](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds.api$s3/679/testReport/junit/org.jclouds.s3.filters/RequestAuthorizeSignatureTest/testIdempotent/) or not...they're certainly not our "usual spurious test failure suspects".
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38274316
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #955](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/955/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38722679
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #691](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/691/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38557724
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #933](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/933/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38274622
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Also thanks from me for the cleanup, @psiniemi! Just a curiosity question: why do we actually need the "connector wrapper" class? Could we configure Guice to try to inject the JSch connector _directly_, or do we need a new instance for every connection? In that case, perhaps make it a factory rather than a provider (minor difference, though).
Since the connector can be "not available", I guess we should be injecting an `Optional<Connector>` rather than a "straight" Connector, though. Then the tests that currently look for `isSshAgentAvailable` could just check whether the Optional is present?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38550385
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -152,7 +163,7 @@ public void clear() {
> }
>
> @Override
> - public Session create() throws Exception {
> + public synchronized Session create() throws Exception {
So if one client is only ever used to create one session, there is no need for synchronization here. Shall I just remove it?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10987347
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #686](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/686/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38469672
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1176](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1176/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38774365
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -152,7 +163,7 @@ public void clear() {
> }
>
> @Override
> - public Session create() throws Exception {
> + public synchronized Session create() throws Exception {
I'd say yes. There is no direct use case in jclouds where a client is
shared between multiple "actors", so I'd just remove it.
Thanks!
El 26/03/2014 18:19, "psiniemi" <no...@github.com> escribió:
> In drivers/jsch/src/main/java/org/jclouds/ssh/jsch/SessionConnection.java:
>
> > @@ -152,7 +163,7 @@ public void clear() {
> > }
> >
> > @Override
> > - public Session create() throws Exception {
> > + public synchronized Session create() throws Exception {
>
> So if one client is only ever used to create one session, there is no need
> for synchronization here. Shall I just remove it?
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/jclouds/jclouds/pull/312/files#r10987347>
> .
>
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10988944
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #692](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/692/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38629249
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
We can depend on core directly, but ```jsch.agentproxy.jsch``` brings in ```com.jcraft.jsch.agentproxy.RemoteIdentityRepository``` which is the glue between the agent and jsch and analogously ```jsch.agentproxy.sshj``` brings in ```com.jcraft.jsch.agentproxy.sshj.AuthAgent``` which is the glue between the agent and sshj.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-39192986
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -156,6 +157,15 @@ public String getPrivateKey() {
> }
>
> /**
> + * @return true if there is a private key attached that is not encrypted
> + */
> + public boolean hasUnencryptedPrivateKey() {
> + return getPrivateKey() != null
> + && !getPrivateKey().isEmpty()
> + && !getPrivateKey().contains(Pems.PROC_TYPE_ENCRYPTED);
Can it be anywhere, or is it actually `startsWith`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923820
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #943](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/943/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38476999
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #958](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/958/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38744806
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #700](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/700/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38724118
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #963](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/963/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38778965
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
> + Connector con = JschSshClient.getSSHAgentConnector();
> + if(con != null ){
[minor] Formatting `if (con != null) {`
And what if con _is_ null? Shouldn't we be going "bang" here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856603
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
> + Connector con = JschSshClient.getSSHAgentConnector();
If we go for an injectable factory/supplier above, inject it here, too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856567
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Now it only has only my commits on top of the fork. Should I try and rebase? The master I last pulled had no conflicts...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38053679
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
Ah, thanks for the explanation. Then the condition we're looking for here is indeed `hasNonencryptedPrivateKey`. What I was trying to get at is that an encrypted private key is still _valid_ (when I hear "not valid", I'm more likely to think "corrupted", "not supported by the target system" etc.), it's just...well...encrypted.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10859015
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Finally! ;-) Committed to [master](https://git-wip-us.apache.org/repos/asf?p=jclouds.git;a=commit;h=85a1a8c1dd3de5f107ddf8e66c22b2fb3410a4ba)
@nacx: Backport to 1.7.x? And many thanks for all your work, @psiniemi!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38777325
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #707 UNSTABLE
Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/707/testReport/junit/org.jclouds.compute.util/ConcurrentOpenSocketFinderTest/testChecksSocketsConcurrently/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38775907
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
There is a branch psiniemi/ssh-agent where these commits are squashed into one. Would it be better to switch to that?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38269365
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Would be nice to get the jenkins conf you run to get these so I could test these myself. Pure mvn clean install gives me no errors.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38277678
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1148](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1148/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38269657
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #923](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/923/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38058229
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #668](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/668/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38037644
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Case insensitive now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38771519
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #934](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/934/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38278456
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -128,23 +132,44 @@ public JschSshClient(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoff
> this.user = checkNotNull(loginCredentials, "loginCredentials").getUser();
> this.host = checkNotNull(socket, "socket").getHostText();
> checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
> - checkArgument(loginCredentials.getPassword() != null || loginCredentials.getPrivateKey() != null,
> - "you must specify a password or a key");
> + checkArgument(loginCredentials.getPassword() != null || hasValidPrivateKey(loginCredentials) || getSSHAgentConnector() != null,
Looking at the code that is actually pretty cheap. The connectors make minimal initialization and do a best effort check to find available agents. Also that is platform specific. The connectors don't have significant resources before querying the keys.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861295
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Ok, I just tested it but I've had to make a few other changes to jclouds to get it working. Here is the test I did:
* Created a new SSH key pair and set a password to the private key.
* Deployed a node in a cloud provider and installed the created public key on it, as follows:
```java
ComputeService compute = context.getComputeService();
TemplateOptions options = compute.templateOptions().authorizePublicKey(publicKey);
compute.createNodesInGroup("ssh", 1, options));
```
* Then tried to run a script on the node (note that the key is still not added to ssh-agent, so this is supposed to fail), as follows:
```java
// Make sure we don't provide a credential so ssh-agent will be used
LoginCredentials creds = LoginCredentials.builder().identity("root").noPassword().noPrivateKey().build();
compute.runScriptOnNode("node-id", Statements.exec("touch /tmp/foo"), overrideLoginCredentials(creds).wrapInInitScript(false));
```
This failed with the following error:
Exception in thread "main" org.jclouds.rest.AuthorizationException: (root:rsa[ssh-agent]@192.241.153.27:22) (root:rsa[ssh-agent]@192.241.153.27:22) error acquiring {hostAndPort=192.241.153.27:22, loginUser=root, ssh=null, connectTimeout=60000, sessionTimeout=60000} (not retryable): Exhausted available authentication methods
Caused by: net.schmizz.sshj.userauth.UserAuthException: Exhausted available authentication methods
... (stacktrace)
Caused by: net.schmizz.sshj.userauth.UserAuthException: publickey auth failed
... (stacktrace)
* Then I added the key to the ssh agent with `ssh-add`.
* Run the same test again and it succeeded! So the `ssh-agent` changes are working as expected.
I've had to change the following, to get it working. jclouds performs some checks to make sure the credentials are present before attempting a connection, and those restrictions have to be relaxed:
* [CreateSshClientOncePortIsListeningOnNode](https://github.com/jclouds/jclouds/blob/master/compute/src/main/java/org/jclouds/compute/functions/CreateSshClientOncePortIsListeningOnNode.java#L68-L69) checks that the credentials are present before creating the SSH client.
* We should also consider removing the check from the [BaseComputeServiceLiveTest.java](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java#L510) class, to allow to run the live tests using the ssh-agent.
* The same should be considered for [this test](https://github.com/jclouds/jclouds/blob/master/compute/src/test/java/org/jclouds/compute/internal/BaseComputeServiceLiveTest.java#L231).
@psiniemi Could you add the mentioned changes to the pull request, so the ssh-agent can be properly used with the Compute Service?
Thanks!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38077166
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #703](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/703/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38746443
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> injector.injectMembers(client);// add logger
> return client;
> }
> }
> + private static class AgentProvider implements Provider<Connector>, SshClient.SshAgentAvailable {
[minor] Add blank line before this class?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923913
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1175](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1175/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38774368
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #678](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/678/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38269362
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #690](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/690/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38555608
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #685](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/685/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38436714
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #706](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/706/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38773956
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -128,23 +132,44 @@ public JschSshClient(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoff
> this.user = checkNotNull(loginCredentials, "loginCredentials").getUser();
> this.host = checkNotNull(socket, "socket").getHostText();
> checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
> - checkArgument(loginCredentials.getPassword() != null || loginCredentials.getPrivateKey() != null,
> - "you must specify a password or a key");
> + checkArgument(loginCredentials.getPassword() != null || hasValidPrivateKey(loginCredentials) || getSSHAgentConnector() != null,
Is there a cheaper way to check for the SSH agent than actually making a connector?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856436
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -156,6 +157,15 @@ public String getPrivateKey() {
> }
>
> /**
> + * @return true if there is a private key attached that is not encrypted
> + */
> + public boolean hasUnencryptedPrivateKey() {
> + return getPrivateKey() != null
> + && !getPrivateKey().isEmpty()
> + && !getPrivateKey().contains(Pems.PROC_TYPE_ENCRYPTED);
The format is not strictly defined, but it's a header and it is safe to assume there may be other headers. At the very least there is the begin marker there before it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924929
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
>The way I, as a user, would like it to work is that I just pile on authentication methods that have some hope of succeeding and then when connecting we run through those until one succeeds or we run out of candidates.
Agree. Upon connection failure, now the drivers retry the connection with the same credentials. Drivers could do it better and try the different auth methods (if there is enough info to try them). In SSHJ there is an explicit auth exception (don't know in JSCH but it should be possible too), so it should be pretty straightforward to implement.
Mind filing a JIRA issue for this? We can address this once this PR has been merged.
>And IMO this pull request represents functionality that is significantly better than having no ssh-agent support.
Absolutely :) I look foward to having this in!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38660756
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
Current implementations of connector do not have any connections outside of the query method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10968348
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1147](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1147/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38268926
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -206,4 +217,13 @@ public String toString() {
> "sessionTimeout", sessionTimeout).toString();
> }
>
> + private static List<AuthMethod> getAuthMethods(AgentProxy agent) throws Exception {
Narrow the list of exception types thrown here?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856725
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Now there is duplicated code in the factories, but I guess they should be totally independent anyways.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38266301
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
Any alternatives for "valid"? What are we looking for here? What is "wrong" about a key containing ""Proc-Type: 4,ENCRYPTED""?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856372
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -58,9 +65,40 @@ public Factory(BackoffLimitedRetryHandler backoffLimitedRetryHandler, Injector i
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new SshjSshClient(backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new SshjSshClient(backoffLimitedRetryHandler, socket, credentials, timeout, injector.getInstance(Connector.class));
Rather than an explicit call to an injector here (which isn't "terrible", I should add), create a method in the module that `@Provides` the connector by creating a new instance of AgentProvider?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924341
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1150](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1150/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38285959
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #689](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/689/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38538809
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Squashed the commits since they didn't add any information and got rid of the last pesky checkstyle violation. Ended up being a missing newline at the end of a file - just couldn't see that in the UI. Had to log into the jenkins server and dig out the xml report to see that.
Anyway this single commit now contains the same rewrite based on the comments from the review. There is some injection and I removed many unnecessary null checks for functions that never return null, which makes the code a lot cleaner.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38535772
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -128,23 +132,44 @@ public JschSshClient(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoff
> this.user = checkNotNull(loginCredentials, "loginCredentials").getUser();
> this.host = checkNotNull(socket, "socket").getHostText();
> checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
> - checkArgument(loginCredentials.getPassword() != null || loginCredentials.getPrivateKey() != null,
> - "you must specify a password or a key");
> + checkArgument(loginCredentials.getPassword() != null || hasValidPrivateKey(loginCredentials) || getSSHAgentConnector() != null,
> + "you must specify a password, a key or an SSH agent needs to be present");
"you must specify a password or a key, or an SSH agent needs to be present"? And would "configured" or "available" be good alternatives for "present"?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856298
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -206,4 +217,13 @@ public String toString() {
> "sessionTimeout", sessionTimeout).toString();
> }
>
> + private static List<AuthMethod> getAuthMethods(AgentProxy agent) throws Exception {
> + Identity[] identities = agent.getIdentities();
> + List<AuthMethod> result = Lists.newArrayList();
> + for (Identity identity : identities) {
> + result.add(new AuthAgent(agent, identity));
> + }
> + return result;
Use a builder? E.g.
```
ImmutableList.Builder<AuthMethod> identities = ImmutableList.builder();
for (Identity identity : agent.getIdentities()) {
identities.add(new AuthAgent(agent, identity));
}
return identities.build();
```
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856774
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1160](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1160/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38556216
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #932](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/932/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38270463
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> I look foward to having this in!
Me too! The updated version looks good to me - just @nacx's question about synchronization to go, I guess? Thanks for making the latest round of changes, @psiniemi!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38684961
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #669 UNSTABLE
Spurious [test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/669/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38061678
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -156,6 +157,15 @@ public String getPrivateKey() {
> }
>
> /**
> + * @return true if there is a private key attached that is not encrypted
> + */
> + public boolean hasUnencryptedPrivateKey() {
> + return getPrivateKey() != null
> + && !getPrivateKey().isEmpty()
> + && !getPrivateKey().contains(Pems.PROC_TYPE_ENCRYPTED);
OK, makes sense then. Do we need a case-insensitive comparison here, perhaps?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10974718
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1173](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1173/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38747539
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -136,10 +156,15 @@ public SSHClient create() throws Exception {
> ssh.connect(hostAndPort.getHostText(), hostAndPort.getPortOrDefault(22));
> if (loginCredentials.getPassword() != null) {
> ssh.authPassword(loginCredentials.getUser(), loginCredentials.getPassword());
> - } else {
> + } else if (loginCredentials.hasUnencryptedPrivateKey()){
[minor] space before `{`.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10922966
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1156](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1156/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38470229
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -128,23 +132,44 @@ public JschSshClient(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoff
> this.user = checkNotNull(loginCredentials, "loginCredentials").getUser();
> this.host = checkNotNull(socket, "socket").getHostText();
> checkArgument(socket.getPort() > 0, "ssh port must be greater then zero" + socket.getPort());
> - checkArgument(loginCredentials.getPassword() != null || loginCredentials.getPrivateKey() != null,
> - "you must specify a password or a key");
> + checkArgument(loginCredentials.getPassword() != null || hasValidPrivateKey(loginCredentials) || getSSHAgentConnector() != null,
> + "you must specify a password, a key or an SSH agent needs to be present");
Maybe "available" would be clearest?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861216
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Oh oh...looks like this [broke Karaf](https://buildhive.cloudbees.com/job/jclouds/job/jclouds-karaf/org.apache.jclouds.karaf$itests/844/testReport/org.jclouds.karaf.itests/MiscFeaturesInstallationTest/testBasicFeaturesInstallation_MiscFeaturesInstallationTest_testBasicFeaturesInstallation_KarafTestContainer_mvn_org_apache_karaf__apache_karaf__2_2_7__tar_gz_/):
```
Caused by: org.osgi.framework.BundleException: Unresolved constraint in bundle jclouds-sshj [84]: Unable to resolve 84.0: missing requirement [84.0] package; (&(package=com.jcraft.jsch.agentproxy)(version>=0.0.0)(!(version>=1.0.0)))
at org.apache.felix.framework.Felix.resolveBundle(Felix.java:3446)
at org.apache.felix.framework.Felix.startBundle(Felix.java:1734)
at org.apache.felix.framework.BundleImpl.start(BundleImpl.java:918)
at org.apache.felix.framework.BundleImpl.start(BundleImpl.java:905)
at org.apache.karaf.features.internal.FeaturesServiceImpl.installFeatures(FeaturesServiceImpl.java:353)
... 61 more
```
@iocanal: Any obvious quick fix here..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-39106796
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -31,7 +31,7 @@
> interface Factory {
>
> SshClient create(HostAndPort socket, LoginCredentials credentials);
> -
> + boolean existsSshAgent();
`hasSshAgent`? Or is the SSH agent not really something the client _has_, rather something it _talks to_?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856250
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1166](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1166/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38656420
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Just seen that the comparison is now case-insensitive. A case-sensitive check, as it was done before, seems more correct. Mind changing this last small bit? I'll merge it afterwards.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38747495
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #928](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/928/) UNSTABLE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38205060
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> injector.injectMembers(client);// add logger
> return client;
> }
> +
> + @Override
> + public boolean existsSshAgent() {
> + try {
Reuse `JschSshClient.getSSHAgentConnector()` here, or the injectable factory/supplier?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856673
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
> + }
> +
> + static Connector getSSHAgentConnector() {
`protected`, in case we ever want to override this?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856446
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
> + }
> +
> + static AgentProxy getSSHAgentProxy() {
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + try {
> + Connector con = cf.createConnector();
> + if (con != null && con.isAvailable()) {
> + return new AgentProxy(con);
> + }
> + } catch (AgentProxyException e) {
> + }
> + return null;
> + }
> +
Same comments here as for the jsch version
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856791
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #928 UNSTABLE
Spurious [test failure](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/org.apache.jclouds$jclouds-core/928/testReport/junit/org.jclouds.rest.functions/PresentWhenApiVersionLexicographicallyAtOrAfterSinceApiVersionTest/testCacheIsFasterWhenNoAnnotationPresent/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38206468
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #904](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/904/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37353896
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1157](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1157/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38476265
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
If I am not wrong, when using encrypted private keys, SSH asks for the encryption password in order to be able to decrypt and use the key. That won't work when connecting programmatically, as the user can't be prompted for the pass. When one adds the key to the `ssh-agent`, though, there is no need to ask for the password, as the agent will take care of handling the authentication. That's why (I think) encrypted keys fallback to the agent by default.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856933
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #677](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/677/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38268628
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
It is ok for me then. Thanks for explaining!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10968324
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> but jsch.agentproxy.jsch brings in com.jcraft.jsch.agentproxy.RemoteIdentityRepository...
I noticed that - sorry, updated the JIRA issue but not this PR ;-) See [SMX4-1728](https://issues.apache.org/jira/browse/SMX4-1728) and [SMX4-1729](https://issues.apache.org/jira/browse/SMX4-1729)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-39194801
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Added the sshj bits. Works for me at least ;)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37317712
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -86,6 +86,16 @@
> <artifactId>jsch</artifactId>
> <scope>compile</scope>
> </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.jsch</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
Ooo...depending on a sub-1.0.0 version? Is that a good idea?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856276
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -31,7 +31,7 @@
> interface Factory {
>
> SshClient create(HostAndPort socket, LoginCredentials credentials);
> -
> + boolean existsSshAgent();
that was copied from the comment above. I guess ```isSshAgentPresent()``` would to me be the clearest.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861188
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + <exception>
> + <conflictingDependencies>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.core</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + </conflictingDependencies>
> + <packages>
> + <package>com.jcraft.jsch.agentproxy</package>
> + </packages>
Do we know what the actual problem is here? Duplicated classes?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856894
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -86,6 +86,16 @@
> <artifactId>jsch</artifactId>
> <scope>compile</scope>
> </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.jsch</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
Thanks for explaining - works for me, then!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10902106
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Sorry @psiniemi I didn't have time to test this. Will do it today!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38036227
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #899](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/899/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37323347
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Let's wait a bit until I test it again and see if @demobox has something to say about the changes. Just to have all the review comments and context in a single place. Then we can close this PR and open a new one from the new branch and merge from there. Does this work for you?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38269640
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
Since Factory is a singleton, then all connections would share the same connector? Would you rather protect against potential concurrency problems only when you run into them?
My use case only uses one ssh connection so right now this would not affect me either way...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10967937
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> Will you backport it too?
I'll submit a PR just to check, yes...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38778047
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1158](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1158/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38534405
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1155](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1155/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38436934
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #648](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/648/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37309725
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #680](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/680/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38285366
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + }
> + @Override
> + public boolean isAvailable() {
> + return false;
> + }
> + @Override
> + public String getName() {
> + return "agent-unavailable";
> + }
> + };
> + }
> + }
> +
> + @Override
> + public boolean isSshAgentAvailable() {
> + return get().isAvailable();
What does JSch expect? That we create a new connector for each connection or can the JSch object be shared, too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924270
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
An empty private key is not going to lead a successful login, so why not try with the agent?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861310
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #941](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/941/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38432822
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
I think the whole authentication login with these ssh connections needs a rethink at some point (hopefully not with this pull request though). The way I, as a user, would like it to work is that I just pile on authentication methods that have some hope of succeeding and then when connecting we run through those until one succeeds or we run out of candidates.
As it currently is implemented, the connections will only try ssh-agent if there is no other method defined.
Also the ```isAvailable()``` logic and how it is used inside the agent proxy librabry is very limited currently. For example on windows that will always return true and this will lead to ```createConnector()``` returning a ```PageantConnector``` regardless of whether one is actually running. Also there is no way of using the OpenSSH ssh-agent on windows currently. I imagine it would require some extra code since on unixes the communication goes over unix sockets with jna native integration. However these are limitations in jsch-agentproxy implementation. And IMO this pull request represents functionality that is significantly better than having no ssh-agent support.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38659488
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
> + Connector con = JschSshClient.getSSHAgentConnector();
> + if(con != null ){
> + IdentityRepository irepo = new RemoteIdentityRepository(con);
> + jsch.setIdentityRepository(irepo);
Inline `irepo`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856612
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> so I'm ok with the current check being case sensitive
Thanks for checking the spec, @nacx. Lazy me :-( Good to go on this one - just the usual squash'n'rebase, and an issue number!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38748053
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1162](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1162/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38629843
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Rebasing to the latest master is something you'll be asked to do before merging. Feel free to rebase now if you want, as if it works when I test it later today I'll go ahead and merge it :)
To avoid this kind of issues, a best practice is to develop the feature in topic branches (and submit the PRs from there) and always keep the "master" branch of the fork unchanged, using it just to sync with the upstream.
Thanks for cleaning up the PR!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38054504
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #947](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/947/) FAILURE
Looks like there's a problem with this pull request
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38556904
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> injector.injectMembers(client);// add logger
> return client;
> }
> +
> + @Override
> + public boolean existsSshAgent() {
> + try {
Could do that (reuse getSSHAgentConnector()), but that would add a dependency to the Client that does not yet exist and would have to make that function public. Easily done if you don't feel that is a problem. To me injection here needlessly complicates things.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861257
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
@nacx: Created [JCLOUDS-516](https://issues.apache.org/jira/browse/JCLOUDS-516) and am about to merge this...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38777027
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #688](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/688/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38534192
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #707](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/707/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38775120
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
Current implementations of connector do not have any connections outside of the query method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10968670
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> Any comments on the dependency thing and where to put the agent detection logic?
Will try to make some time to review this later...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38301606
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #653](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/653/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37352984
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #696](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/696/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38656044
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
Guess that's true.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861221
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -86,6 +86,16 @@
> <artifactId>jsch</artifactId>
> <scope>compile</scope>
> </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.jsch</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
Jcraft has their own policy with the version numbering. Practically all of their stuff is sub 1.0.0 (except for jzlib). My experience with their quality and stability has been good. The project is a couple of years old, hosted on github: https://github.com/ymnk/jsch-agent-proxy , and in actual use by several projects.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861211
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
> + }
> +
> + static Connector getSSHAgentConnector() {
> + JSch.setConfig("PreferredAuthentications", "publickey");
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + try {
> + Connector con = cf.createConnector();
> + if (con != null && con.isAvailable()) {
> + return con;
> + }
> + } catch (AgentProxyException e) {
Do we want to swallow this, or at least warn somewhere?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856480
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> this.hostAndPort = checkNotNull(hostAndPort, "hostAndPort");
> this.loginCredentials = checkNotNull(loginCredentials, "loginCredentials for %", hostAndPort);
> this.connectTimeout = connectTimeout;
> this.sessionTimeout = sessionTimeout;
> this.proxy = checkNotNull(proxy, "proxy for %", hostAndPort);
> + this.agentConnector = agentConnector;
Can this be null? Otherwise add `checkNotNull`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923869
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1138](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1138/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38038330
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @psiniemi!
Good to go @demobox! I'd like to have this also in 1.7.x. Will you backport it too?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38777449
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
> + Connector con = JschSshClient.getSSHAgentConnector();
Where should that code go? Interfaces in org.jclouds.compute.ssh.SshClient and analogous implementations in packages. I'll try that out, but that will result in having the jcraft-agentproxy-core dependency in jclouds-compute
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861791
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Uh sorry, we use our fork for our development so I pulled in the current jclouds master. Didn't realized they would come into this pull request... I'll try and clean that up.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38050175
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -160,11 +171,14 @@ public Session create() throws Exception {
> session.setTimeout(sessionTimeout);
> if (loginCredentials.getPrivateKey() == null) {
> session.setPassword(loginCredentials.getPassword());
> - } else {
> - checkArgument(!loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED"),
Same question as above...contains or startsWith?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923879
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #687](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/687/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38475747
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> + return ConnectorFactory.getDefault().createConnector();
> + } catch (final AgentProxyException e) {
> + return new Connector() {
> + @Override
> + public void query(Buffer buffer) throws AgentProxyException {
> + throw new AgentProxyException("SSH Agent not available: " + e.getMessage());
> + }
> + @Override
> + public boolean isAvailable() {
> + return false;
> + }
> + @Override
> + public String getName() {
> + return "agent-unavailable";
> + }
> + };
Fix indentation?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10922887
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> Now it only has only my commits on top of the fork.
Great! This is fine for now. We'll rebase just before merging, most likely...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38054543
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #952](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/952/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38657437
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Sure. I don't want that to block this PR, just wanted to point it out, as you might be already familiar with the library and perhaps able to add support for sshj faster than anyone else :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37312632
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
I just tested it again and it works great. Thanks @psiniemi !
@demobox Any comments on the dependency thing and where to put the agent detection logic?
@psiniemi when everything is OK I'll squash and push the commits myself, so you don't have to open a new PR from your other branch (let's get this in! :D). Agree?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38290268
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds » jclouds #934 FAILURE
Another [GitHub timeout](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/934/console)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38278954
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
>Now there is duplicated code in the factories, but I guess they should be totally independent anyways.
I personally prefer having that small bit of code duplicated in the different SSH drivers that use the lib than having the compute project coupled to it. @demobox thoughts on this?
I'll give it a try again today. Thanks for the fixes @psiniemi !
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38268961
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -62,5 +65,19 @@ public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> injector.injectMembers(client);// add logger
> return client;
> }
> +
> + @Override
> + public boolean existsSshAgent() {
> + try {
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + if (cf != null) {
> + Connector con = cf.createConnector();
> + if (con != null) {
> + return con.isAvailable();
> + }
> + }
> + } catch (AgentProxyException e) {}
> + return false;
> + }
Same comments as for the jsch version
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856797
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
> + }
> +
> + static Connector getSSHAgentConnector() {
> + JSch.setConfig("PreferredAuthentications", "publickey");
Hm...should we be doing this here, or earlier (and then once, hopefully), or when we try to connect?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856466
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -176,6 +201,14 @@ public int getSessionTimeout() {
> }
>
> /**
> + *
> + * @return Ssh agent connector
> + */
> + public Connector getAgentConnector() {
> + return agentConnector;
[minor] Indentation
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924291
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Sounds great :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38292692
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Awesome. Will try both today! Thanks @psiniemi!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37318216
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> injector.injectMembers(client);// add logger
> return client;
> }
> +
> + @Override
> + public boolean existsSshAgent() {
> + try {
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + if (cf != null) {
> + Connector con = cf.createConnector();
> + if (con != null) {
> + return con.isAvailable();
> + }
> + }
> + } catch (AgentProxyException e) {}
> + return false;
Put this in the catch block, to make it clear this is only the result if something goes wrong?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856630
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1170](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1170/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38724813
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1118](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1118/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37310528
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #946](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/946/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38554869
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #705](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/705/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38773936
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #945](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/945/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38538216
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Thanks for this submission, @psiniemi! I think something went wrong with a recent update? There are now lots of commits and changes in this PR that don't seem to be related to your work. Could you rebase and update?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38048975
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
I suspect it's spurious, since previous builds didn't fail. And you should be able to access the Jenkins logs - this is a public URL:
https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/679/consoleFull
In any case, I'll close'n'reopen to see if the failures go away.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38278109
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1123](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1123/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37353112
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1143](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1143/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38207968
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #948](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/948/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38628149
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
Current implementations of connector do not have any connections outside of the query method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10968271
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #944](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/944/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38533726
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -152,7 +163,7 @@ public void clear() {
> }
>
> @Override
> - public Session create() throws Exception {
> + public synchronized Session create() throws Exception {
Just looked inside the code for the connectors and the connectors have native libraries as instance variables without any locks.
The libraries are pretty basic so I guess they may be thread safe, but for windows I have no way of testing this.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10967742
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
Reuse the `agentConnector` variable, as it is already initialized?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10967539
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
>Does that make some kind of sense? @nacx: thoughts on this?
Yep. I think it would be cleaner. Returning a "fake" agent relies on clients calling the "isAvailable" method to know if the agent is there, and there is no practical way to prevent them from calling the "query" method that will always fail. Returning an `Optional` seems more correct here.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38559089
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1139](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1139/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38061800
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
>Only the minor question about a possible case-insensitive check for the "encrypted header" from me
@demobox Reading [the spec of the Proc-Type header](http://tools.ietf.org/html/rfc1421#section-4.6.1.1) it seems that the possible values are fixed and uppercase, so I'm ok with the current check being case sensitive. If you're ok too I'll merge it in a while.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38746959
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
Simply `} else if {`?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923890
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> @@ -67,5 +70,19 @@ public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> injector.injectMembers(client);// add logger
> return client;
> }
> +
> + @Override
> + public boolean existsSshAgent() {
> + try {
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + if (cf != null) {
> + Connector con = cf.createConnector();
> + if (con != null) {
> + return con.isAvailable();
> + }
> + }
> + } catch (AgentProxyException e) {}
> + return false;
Would need to add it into all of the else blocks as well
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861235
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1161](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1161/) FAILURE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38556586
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1177](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1177/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38775448
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
I don't get these errors, but the injection doesn't work. My bad. Will fix...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38441325
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Thanks for the cleanup and the squash @psiniemi! Code looks much more clean now, and like the `AgentProvider approach`. Just a couple minors about style and one comment about the connector configuration, but nice cleanup!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38546804
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
Why the empty check? This is new, I guess? Previously, an empty (but non-null) private key would have been accepted by the code?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856393
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
So it seems that the two deps `jsch.agentproxy.jsch` and `jsch.agentproxy.sshj` are actually just aggregators that include `jsch.agentproxy.core` and jsch/sshj, respectively.
@psiniemi: Is there any reason not to simply depend on `jsch.agentproxy.core` directly, since we import jsch/sshj in the driver projects already? The reason for asking is that `jsch.agentproxy.core` is an OSGi bundle, but the other two are not.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-39183652
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Only the minor question about a possible case-insensitive check for the "encrypted header" from me, otherwise +1 - looks good. @nacx: Any more questions from you?
I guess we want want to backport this to 1.7.x? And we should probably create an issue for this, just for housekeeping...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38737396
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
> + }
> +
> + static Connector getSSHAgentConnector() {
> + JSch.setConfig("PreferredAuthentications", "publickey");
> + ConnectorFactory cf = ConnectorFactory.getDefault();
> + try {
> + Connector con = cf.createConnector();
> + if (con != null && con.isAvailable()) {
> + return con;
> + }
> + } catch (AgentProxyException e) {
> + }
> + return null;
> + }
This method will be really nasty to mock due to the statics. Should we make this an injectable factory instead?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856513
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> }
> sessionConnection = SessionConnection.builder().hostAndPort(HostAndPort.fromParts(host, socket.getPort())).loginCredentials(
> loginCredentials).proxy(checkNotNull(proxyConfig, "proxyConfig")).connectTimeout(timeout).sessionTimeout(timeout).build();
> }
>
> + static boolean hasValidPrivateKey(LoginCredentials loginCredentials) {
> + return loginCredentials.getPrivateKey() != null &&
> + !loginCredentials.getPrivateKey().isEmpty() &&
> + !loginCredentials.getPrivateKey().contains("Proc-Type: 4,ENCRYPTED");
Make `"Proc-Type: 4,ENCRYPTED"` a constant with a descriptive name?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856402
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -122,15 +127,21 @@ public Builder from(SessionConnection in) {
> .connectTimeout(in.connectTimeout).sessionTimeout(in.sessionTimeout);
> }
>
> + public Builder agentConnector(Connector agentConnector) {
> + this.agentConnector = agentConnector;
> + return this;
> + }
Properly indent this to 3 spaces.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10922879
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> + }
> + @Override
> + public boolean isAvailable() {
> + return false;
> + }
> + @Override
> + public String getName() {
> + return "agent-unavailable";
> + }
> + };
> + }
> + }
> +
> + @Override
> + public boolean isSshAgentAvailable() {
> + return get().isAvailable();
Is it ok to always call `get()` and execute the connector creation logic? As the `Connector` provider is defined as singleton perhaps this should be a different class that uses the injected Connector instead of building a new one each time?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10922932
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1119](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1119/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37325515
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> byte[] privateKey = loginCredentials.getPrivateKey().getBytes();
> jsch.addIdentity(loginCredentials.getUser(), privateKey, null, emptyPassPhrase);
> + } else {
The checkArgument above makes the resulting else branch impossible to reach...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10956917
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #680 SUCCESS
> jclouds-java-7-pull-requests #1150 SUCCESS
Bingo ;-)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38286311
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #649](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/649/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37324919
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #679](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/679/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38273093
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1149](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1149/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38273580
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
> + <exception>
> + <conflictingDependencies>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.core</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + </conflictingDependencies>
> + <packages>
> + <package>com.jcraft.jsch.agentproxy</package>
> + </packages>
Yes, duplicated classes that are identical
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10861681
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
Closed #312.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -63,9 +77,14 @@ public Factory(ProxyConfig proxyConfig, BackoffLimitedRetryHandler backoffLimite
>
> @Override
> public SshClient create(HostAndPort socket, LoginCredentials credentials) {
> - SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout);
> + SshClient client = new JschSshClient(proxyConfig, backoffLimitedRetryHandler, socket, credentials, timeout, getAgentConnector());
I see. So there is one new connector that is passed to each client, and one singleton connector that is used to check the presence of the agent.
I'm fine with this, but does the `Connector` open a connection or does it allocate resources we would want to close once we know it is available? I mean if it does keep a connection or similar, perhaps it is better to just keep an `isAvailable` boolean variable instead of the singleton Connector.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10968181
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + public boolean isAvailable() {
> + return false;
> + }
> + @Override
> + public String getName() {
> + return "agent-unavailable";
> + }
> + };
> + }
> + }
> +
> + @Override
> + public boolean isSshAgentAvailable() {
> + return get().isAvailable();
> + }
> + }
Same comment as for the other variant above. And I guess there's little way to avoid duplicating this class in both places..?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924383
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> jclouds-pull-requests #673 UNSTABLE
Another [spurious test failure](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/org.apache.jclouds$jclouds-compute/673/testReport/junit/org.jclouds.compute.callables/BlockUntilInitScriptStatusIsZeroThenReturnOutputTest/testloopUntilTrueOrThrowCancellationExceptionReturnsWhenPredicateIsTrueSecondTimeWhileNotCancelled/)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38208199
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by BuildHive <no...@github.com>.
[jclouds » jclouds #922](https://buildhive.cloudbees.com/job/jclouds/job/jclouds/922/) SUCCESS
This pull request looks good
[(what's this?)](https://www.cloudbees.com/what-is-buildhive)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38036047
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #669](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/669/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38060588
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> injector.injectMembers(client);// add logger
> return client;
> }
> }
> + private static class AgentProvider implements Provider<Connector>, SshClient.SshAgentAvailable {
> +
> + @Override
> + public Connector get() {
> + try {
> + return ConnectorFactory.getDefault().createConnector();
> + } catch (final AgentProxyException e) {
> + return new Connector() {
Rather than returning a "dummy connector", make this a provider of an `Optional<Connector>` (`null` is obviously not "allowed" as a return value for a Provider)?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10924240
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -86,6 +86,16 @@
> <artifactId>jsch</artifactId>
> <scope>compile</scope>
> </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.jsch</artifactId>
> + <version>0.0.7</version>
> + </dependency>
Do we need this, since the sshj version only needs core?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10856861
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @psiniemi! This looks like great stuff. I'll give it a try in short!
BTW, do you know if this ssh-agent stuff can also be applied to the `jclouds-sshj` driver by using the [sshj integration](https://github.com/ymnk/jsch-agent-proxy/tree/master/jsch-agent-proxy-sshj)? AFAIK, the sshj driver is the one that is more often used, and it would be great to support the ssh-agent on it too.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37303131
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
I can probably make another pull request for the agent support for sshj. There is at least an example of sshj using the same agent proxy.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37311936
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-java-7-pull-requests #1159](https://jclouds.ci.cloudbees.com/job/jclouds-java-7-pull-requests/1159/) SUCCESS
This pull request looks good
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38539081
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -152,7 +163,7 @@ public void clear() {
> }
>
> @Override
> - public Session create() throws Exception {
> + public synchronized Session create() throws Exception {
Have you experienced concurrency issues? In theory, the ssh client factory will create independent clients, and each one will have its own connection. Is there any use case where the same client and connection is used simultaneously by different users/clients/actors? (Such use case does not exist in jclouds-compute).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10967503
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> + <exception>
> + <conflictingDependencies>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.core</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + <dependency>
> + <groupId>com.jcraft</groupId>
> + <artifactId>jsch.agentproxy.connector-factory</artifactId>
> + <version>0.0.7</version>
> + </dependency>
> + </conflictingDependencies>
> + <packages>
> + <package>com.jcraft.jsch.agentproxy</package>
> + </packages>
OK, thanks. Then fine with the exception.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10974759
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Started this in the branch. Will finish and merge tonight.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38568968
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by Andrew Phillips <no...@github.com>.
> @@ -34,6 +34,12 @@
>
> }
>
> + interface SshAgentAvailable {
> +
> + boolean isSshAgentAvailable();
> +
> + }
[minor] Remove blank lines and rename to something like `SshAgentFeatureFlag` or `SshClientFeatures` or so?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312/files#r10923788
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by psiniemi <no...@github.com>.
Might be worth mentioning that I had no way of testing this on a windows client. Pageant should work, but I have no way of verifying that...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-37329118
Re: [jclouds] Add ssh-agent support via jsch agentproxy (#312)
Posted by CloudBees pull request builder plugin <no...@github.com>.
[jclouds-pull-requests #673](https://jclouds.ci.cloudbees.com/job/jclouds-pull-requests/673/) UNSTABLE
Looks like there's a problem with this pull request
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds/pull/312#issuecomment-38206878