You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2018/05/24 10:07:23 UTC
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/963
disable jclouds authorizePublicKey for extra ssh public key data
we install this ourselves in addition so no need, and if we ask jclouds to do it, it seems to break sometimes (eg when keyPair is set).
there is a system property that can be set to revert for legacy behaviour.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server fix-extra-ssh-auth-bug
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/963.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 #963
----
commit 1950baaec5e3be4a69382eeb91ce132257dc9129
Author: Alex Heneveld <al...@...>
Date: 2018-05-24T10:03:38Z
disable jclouds authorizePublicKey for extra ssh public key data
we install this ourselves in addition so no need, and if we ask jclouds to do it, it seems to break sometimes (eg when keyPair is set).
there is a system property that can be set to revert for legacy behaviour.
----
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190576983
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AwsEc2LocationLiveTest.java ---
@@ -28,6 +30,8 @@
*/
public class AwsEc2LocationLiveTest extends AbstractJcloudsLocationTest {
+ private static final Logger log = LoggerFactory.getLogger(AwsEc2LocationLiveTest.class);
--- End diff --
The logger is not used here. Can be removed
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190597772
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AwsEc2LocationLiveTest.java ---
@@ -68,4 +72,29 @@ public AwsEc2LocationLiveTest() {
@Test(enabled = false)
public void noop() { } /* just exists to let testNG IDE run the test */
+
+ // outline of test which can be used to assert combos of keys etc
+ // currently uses my (alex's) hardcoded keys and preexisting pair;
+ // should refactor to generate keys and the keypair and make a real test,
+ // but for now keeping it commented out as a template for people to do manual testing
+// @Test(groups = "Live")
+// public void testProvisionVmAndAuthPublicKey() {
+// String regionName = USEAST_REGION_NAME;
+// loc = (JcloudsLocation) mgmt().getLocationRegistry().getLocationManaged(provider + (regionName == null ? "" : ":" + regionName));
+// SshMachineLocation machine = obtainMachine(MutableMap.of(
+// JcloudsLocationConfig.KEY_PAIR, "alex-aws-2",
+// JcloudsLocationConfig.LOGIN_USER_PRIVATE_KEY_FILE, "~/.ssh/alex-aws-2-id_rsa",
+//
+// JcloudsLocationConfig.PRIVATE_KEY_FILE, "~/.ssh/aws-cloudera_rsa",
+// JcloudsLocationConfig.PUBLIC_KEY_FILE, "~/.ssh/aws-cloudera_rsa.pub",
+// JcloudsLocationConfig.EXTRA_PUBLIC_KEY_DATA_TO_AUTH,
+// aws-whirr
+// "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAw9R7FG0pOpZ7MR+KYty+UzHerEtW9NVBJj+NmznT4zg57klqDAxitahe6cJpj8Nbt0Rmp9G4TZQzAWuoSH5ZUJpFpcVP75tMYWOP6ZymH7MZ5hXLjLHTicNyQ/EB9H2eXSK2xLnq/8oDnzKgnhIXL7tbcC7hOY9Yzu25UrO+xQDbzM3nuwlr38JJwo1fLsIiEVI3uutZW9ANZgcZg0USlFFvxGcAA2KZ322tqtQtP3YYE0IogYUjTSFj5xexFDzIcN5V2Z2tHYKW+Jl/jR98EAsq4By1L+whoX142NJGZsB1GKm4zZTh3vjfzpeGNmxrrHDGp2TOCGFJjk2seHqyyw== alex@almac.rocklynn.cognetics.org",
+// JcloudsLocationConfig.IMAGE_ID, "us-east-1/ami-5492ba3c"
+// ));
+//
+// log.info("Provisioned {} vm {}; checking if ssh'able", provider, machine);
+// assertTrue(machine.isSshable());
+// }
--- End diff --
nice! i've added this and will merge as soon as it passes
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by tbouron <gi...@git.apache.org>.
Github user tbouron commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190576874
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AbstractJcloudsLocationTest.java ---
@@ -48,7 +48,7 @@
public abstract class AbstractJcloudsLocationTest {
private static final Logger LOG = LoggerFactory.getLogger(AbstractJcloudsLocationTest.class);
- private final String provider;
+ protected final String provider;
--- End diff --
Why making this field `protected` instead of `private`. Doesn't seem to be used elsewhere, at least in this PR
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190597597
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AbstractJcloudsLocationTest.java ---
@@ -48,7 +48,7 @@
public abstract class AbstractJcloudsLocationTest {
private static final Logger LOG = LoggerFactory.getLogger(AbstractJcloudsLocationTest.class);
- private final String provider;
+ protected final String provider;
--- End diff --
used in test (which is now added)
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by andreaturli <gi...@git.apache.org>.
Github user andreaturli commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190581942
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AwsEc2LocationLiveTest.java ---
@@ -68,4 +72,29 @@ public AwsEc2LocationLiveTest() {
@Test(enabled = false)
public void noop() { } /* just exists to let testNG IDE run the test */
+
+ // outline of test which can be used to assert combos of keys etc
+ // currently uses my (alex's) hardcoded keys and preexisting pair;
+ // should refactor to generate keys and the keypair and make a real test,
+ // but for now keeping it commented out as a template for people to do manual testing
+// @Test(groups = "Live")
+// public void testProvisionVmAndAuthPublicKey() {
+// String regionName = USEAST_REGION_NAME;
+// loc = (JcloudsLocation) mgmt().getLocationRegistry().getLocationManaged(provider + (regionName == null ? "" : ":" + regionName));
+// SshMachineLocation machine = obtainMachine(MutableMap.of(
+// JcloudsLocationConfig.KEY_PAIR, "alex-aws-2",
+// JcloudsLocationConfig.LOGIN_USER_PRIVATE_KEY_FILE, "~/.ssh/alex-aws-2-id_rsa",
+//
+// JcloudsLocationConfig.PRIVATE_KEY_FILE, "~/.ssh/aws-cloudera_rsa",
+// JcloudsLocationConfig.PUBLIC_KEY_FILE, "~/.ssh/aws-cloudera_rsa.pub",
+// JcloudsLocationConfig.EXTRA_PUBLIC_KEY_DATA_TO_AUTH,
+// aws-whirr
+// "ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAw9R7FG0pOpZ7MR+KYty+UzHerEtW9NVBJj+NmznT4zg57klqDAxitahe6cJpj8Nbt0Rmp9G4TZQzAWuoSH5ZUJpFpcVP75tMYWOP6ZymH7MZ5hXLjLHTicNyQ/EB9H2eXSK2xLnq/8oDnzKgnhIXL7tbcC7hOY9Yzu25UrO+xQDbzM3nuwlr38JJwo1fLsIiEVI3uutZW9ANZgcZg0USlFFvxGcAA2KZ322tqtQtP3YYE0IogYUjTSFj5xexFDzIcN5V2Z2tHYKW+Jl/jR98EAsq4By1L+whoX142NJGZsB1GKm4zZTh3vjfzpeGNmxrrHDGp2TOCGFJjk2seHqyyw== alex@almac.rocklynn.cognetics.org",
+// JcloudsLocationConfig.IMAGE_ID, "us-east-1/ami-5492ba3c"
+// ));
+//
+// log.info("Provisioned {} vm {}; checking if ssh'able", provider, machine);
+// assertTrue(machine.isSshable());
+// }
--- End diff --
I took a first stab at it
```
@Test(groups = "Live")
public void testProvisionVmAndAuthPublicKey() {
String regionName = USEAST_REGION_NAME;
loc = (JcloudsLocation) mgmt().getLocationRegistry().getLocationManaged(provider + (regionName == null ? "" : ":" + regionName));
EC2Api api = loc.getComputeService().getContext().unwrapApi(EC2Api.class);
String primaryKeyName = "brooklyn-keypair-" + System.currentTimeMillis();
try {
Map<String, String> extra = SshKeys.generate();
KeyPair primary = api.getKeyPairApiForRegion(regionName).get().createKeyPairInRegion(regionName, primaryKeyName);
SshMachineLocation machine = obtainMachine(MutableMap.of(
JcloudsLocationConfig.KEY_PAIR, primary.getKeyName(),
JcloudsLocationConfig.LOGIN_USER_PRIVATE_KEY_DATA, primary.getKeyMaterial(),
JcloudsLocationConfig.EXTRA_PUBLIC_KEY_DATA_TO_AUTH, extra.get("public"),
JcloudsLocationConfig.IMAGE_ID, "us-east-1/ami-5492ba3c"
));
log.info("Provisioned {} vm {}; checking if ssh'able", provider, machine);
assertTrue(machine.isSshable());
} finally {
api.getKeyPairApiForRegion(USEAST_REGION_NAME).get().deleteKeyPairInRegion(USEAST_REGION_NAME, primaryKeyName);
}
}
```
tested on my aws account and seems fine. Are you happy with that @ahgittin ? if so, can you please enable it instead of the commented out test? thanks
---
[GitHub] brooklyn-server issue #963: disable jclouds authorizePublicKey for extra ssh...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/963
test works perfectly -- very very nice @andreaturli !
merging
---
[GitHub] brooklyn-server issue #963: disable jclouds authorizePublicKey for extra ssh...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/963
note the code at https://github.com/apache/brooklyn-server/blob/master/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/JcloudsLocation.java#L1037 is unchanged, this is where we (brooklyn) install the supplied data to `authorized_keys` ourselves, so disabling jclouds seems save
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/963
---
[GitHub] brooklyn-server pull request #963: disable jclouds authorizePublicKey for ex...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/963#discussion_r190597658
--- Diff: locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/provider/AwsEc2LocationLiveTest.java ---
@@ -28,6 +30,8 @@
*/
public class AwsEc2LocationLiveTest extends AbstractJcloudsLocationTest {
+ private static final Logger log = LoggerFactory.getLogger(AwsEc2LocationLiveTest.class);
--- End diff --
also used in test
---