You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by bostko <gi...@git.apache.org> on 2015/11/27 00:32:52 UTC
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
GitHub user bostko opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/1074
Add documentation and additional tests to SshToolAbstractIntegrationTest
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/bostko/incubator-brooklyn ssh_tool_abstract_test
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/1074.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1074
----
commit 16880bbd92f0744b177f9bb4da2fcd12cd404aec
Author: Valentin Aitken <va...@cloudsoftcorp.com>
Date: 2015-11-26T21:56:10Z
Add documentation and additional tests to SshToolAbstractIntegrationTest
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/1074#issuecomment-160093363
looks good, merging (but please open the jira issue)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
Posted by bostko <gi...@git.apache.org>.
Github user bostko commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1074#discussion_r46010405
--- Diff: core/src/test/java/org/apache/brooklyn/util/core/internal/ssh/SshToolAbstractIntegrationTest.java ---
@@ -264,8 +271,44 @@ public void testSshKeyWithPassphrase() throws Exception {
}
@Test(groups = {"Integration"})
+ public void testSshKeyWithNoKeyDefaultsToIdrsa() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .build());
+ tools.add(localtool);
+ localtool.connect();
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+ }
+
+ @Test(groups = {"Integration"})
+ public void testSshKeyWithPrivateKeyData() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), new String(Files.toByteArray(new File(Os.tidyPath(SSH_DEFAULT_KEYFILE))), StandardCharsets.UTF_8))
+ .build());
+ localtool.connect();
+
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+
+ // Also needs the negative test to prove that we're really using an ssh-key with a passphrase
+ try {
+ final SshTool localtool2 = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), "invalid data")
+ .build());
+ localtool2.connect();
+ localtool2.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date"));
--- End diff --
@aledsage this was succeeding for me. I found that I have to use "IdentitiesOnly=yes" in order to deny ssh to try keys other than the specified keys as a parameter or from the config file.
Check http://linux.die.net/man/5/ssh_config
Do you think it is reasonable make a configurable IdentitiesOnly parameter in the SshTool in order to use it for such tests?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1074#discussion_r46030279
--- Diff: core/src/test/java/org/apache/brooklyn/util/core/internal/ssh/SshToolAbstractIntegrationTest.java ---
@@ -264,8 +271,44 @@ public void testSshKeyWithPassphrase() throws Exception {
}
@Test(groups = {"Integration"})
+ public void testSshKeyWithNoKeyDefaultsToIdrsa() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .build());
+ tools.add(localtool);
+ localtool.connect();
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+ }
+
+ @Test(groups = {"Integration"})
+ public void testSshKeyWithPrivateKeyData() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), new String(Files.toByteArray(new File(Os.tidyPath(SSH_DEFAULT_KEYFILE))), StandardCharsets.UTF_8))
+ .build());
+ localtool.connect();
+
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+
+ // Also needs the negative test to prove that we're really using an ssh-key with a passphrase
+ try {
+ final SshTool localtool2 = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), "invalid data")
+ .build());
+ localtool2.connect();
+ localtool2.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date"));
+ // Notice that executing a command may succeed for SshCliToolIntegrationTest.testSshKeyWithPrivateKeyData if you already have valid keys loaded in the ssh-agent
+ fail();
--- End diff --
you can use `Asserts.shouldHaveFailedPreviously()` then `Asserts.expectedFailureContains(SshException.class.getSimpleName())` for a more readable test.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/1074#discussion_r46030081
--- Diff: core/src/test/java/org/apache/brooklyn/util/core/internal/ssh/SshToolAbstractIntegrationTest.java ---
@@ -264,8 +271,44 @@ public void testSshKeyWithPassphrase() throws Exception {
}
@Test(groups = {"Integration"})
+ public void testSshKeyWithNoKeyDefaultsToIdrsa() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .build());
+ tools.add(localtool);
+ localtool.connect();
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+ }
+
+ @Test(groups = {"Integration"})
+ public void testSshKeyWithPrivateKeyData() throws Exception {
+ final SshTool localtool = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), new String(Files.toByteArray(new File(Os.tidyPath(SSH_DEFAULT_KEYFILE))), StandardCharsets.UTF_8))
+ .build());
+ localtool.connect();
+
+ assertEquals(localtool.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date")), 0);
+
+ // Also needs the negative test to prove that we're really using an ssh-key with a passphrase
+ try {
+ final SshTool localtool2 = newTool(ImmutableMap.<String,Object>builder()
+ .put(SshTool.PROP_HOST.getName(), "localhost")
+ .put(SshTool.PROP_PRIVATE_KEY_DATA.getName(), "invalid data")
+ .build());
+ localtool2.connect();
+ localtool2.execScript(MutableMap.<String,Object>of(), ImmutableList.of("date"));
--- End diff --
@bostko i've never encountered a situation where the default `identitiesOnly=no` causes a problem. can you tell what settings (presumably in the `sshd` config on your localhost) make this necessary?
for integration tests on localhost you should be able to undo those.
we could allow arbitrary tool-specific ssh-tool configuration to be specified in `brooklyn.properties` in the first instance, only introducing an `identitiesOnly=yes` parameter if the issue recurs.
definitely open a jira issue in case other people hit this, including info on the ssh client and server config, and the errors which result.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Add documentation and additional ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/1074
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---