From 9104eca2827a81d975531f03ff2b66b72082286c Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Fri, 14 Oct 2011 01:49:18 -0700 Subject: [PATCH] Issue 717: corrected keyPair state logic which was redundantly creating keys --- .../ec2/compute/EC2ComputeService.java | 12 +-- .../EC2ComputeServiceDependenciesModule.java | 9 ++- .../functions/CreateUniqueKeyPair.java | 8 +- .../functions/CredentialsForInstance.java | 5 +- ...rityGroupsAsNeededAndReturnRunOptions.java | 51 +++++++----- ...GroupsAsNeededAndReturnRunOptionsTest.java | 27 ++++--- .../main/java/org/jclouds/crypto/SshKeys.java | 10 +++ .../aws/ec2/compute/AWSEC2ComputeService.java | 5 +- ...WSEC2ComputeServiceDependenciesModule.java | 2 +- ...rityGroupsAsNeededAndReturnRunOptions.java | 26 ++++--- .../ImportOrReturnExistingKeypair.java | 18 ++++- ...GroupsAsNeededAndReturnRunOptionsTest.java | 78 +++++++------------ .../ImportOrReturnExistingKeypairTest.java | 43 +++++----- .../services/AWSKeyPairClientLiveTest.java | 5 +- 14 files changed, 161 insertions(+), 138 deletions(-) diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java index 956e65c163..aa11418e94 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/EC2ComputeService.java @@ -24,8 +24,9 @@ import static com.google.common.collect.Iterables.transform; import static org.jclouds.util.Preconditions2.checkNotEmpty; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import javax.inject.Inject; @@ -70,8 +71,8 @@ import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.cache.Cache; import com.google.common.collect.ImmutableMultimap; -import com.google.common.collect.ImmutableMultimap.Builder; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableMultimap.Builder; /** * @author Adrian Cole @@ -79,7 +80,7 @@ import com.google.common.collect.ImmutableSet; @Singleton public class EC2ComputeService extends BaseComputeService { private final EC2Client ec2Client; - private final Cache credentialsMap; + private final ConcurrentMap credentialsMap; private final Cache securityGroupMap; @Inject @@ -97,7 +98,7 @@ public class EC2ComputeService extends BaseComputeService { RunScriptOnNode.Factory runScriptOnNodeFactory, InitAdminAccess initAdminAccess, PersistNodeCredentials persistNodeCredentials, Timeouts timeouts, @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor, EC2Client ec2Client, - Cache credentialsMap, @Named("SECURITY") Cache securityGroupMap) { + ConcurrentMap credentialsMap, @Named("SECURITY") Cache securityGroupMap) { super(context, credentialStore, images, sizes, locations, listNodesStrategy, getNodeMetadataStrategy, runNodesAndAddToSetStrategy, rebootNodeStrategy, destroyNodeStrategy, startNodeStrategy, stopNodeStrategy, templateBuilderProvider, templateOptionsProvider, nodeRunning, nodeTerminated, nodeSuspended, @@ -143,7 +144,8 @@ public class EC2ComputeService extends BaseComputeService { logger.debug(">> deleting keyPair(%s)", keyPair.getKeyName()); ec2Client.getKeyPairServices().deleteKeyPairInRegion(region, keyPair.getKeyName()); // TODO: test this clear happens - credentialsMap.invalidate(new RegionAndName(region, keyPair.getKeyName())); + credentialsMap.remove(new RegionAndName(region, keyPair.getKeyName())); + credentialsMap.remove(new RegionAndName(region, group)); logger.debug("<< deleted keyPair(%s)", keyPair.getKeyName()); } } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java index ec829a2aa6..290e651912 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/config/EC2ComputeServiceDependenciesModule.java @@ -22,6 +22,7 @@ import static org.jclouds.ec2.reference.EC2Constants.PROPERTY_EC2_TIMEOUT_SECURI import java.security.SecureRandom; import java.util.Map; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; import javax.inject.Named; @@ -62,7 +63,9 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import com.google.common.cache.CacheLoader; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import com.google.inject.AbstractModule; +import com.google.inject.Injector; import com.google.inject.Provides; import com.google.inject.Scopes; import com.google.inject.TypeLiteral; @@ -97,7 +100,7 @@ public class EC2ComputeServiceDependenciesModule extends AbstractModule { }).to(CredentialsForInstance.class); bind(new TypeLiteral>() { }).to(CreateSecurityGroupIfNeeded.class); - bind(new TypeLiteral>() { + bind(new TypeLiteral>() { }).to(CreateUniqueKeyPair.class); bind(new TypeLiteral>() { }).to(RegionAndIdToImage.class); @@ -131,8 +134,8 @@ public class EC2ComputeServiceDependenciesModule extends AbstractModule { @Provides @Singleton - protected Cache keypairMap(CacheLoader in) { - return CacheBuilder.newBuilder().build(in); + protected ConcurrentMap keypairMap(Injector i) { + return Maps.newConcurrentMap(); } @Provides diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java index 85d5e99e65..c97094f6e5 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CreateUniqueKeyPair.java @@ -32,15 +32,15 @@ import org.jclouds.ec2.domain.KeyPair; import org.jclouds.logging.Logger; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Supplier; -import com.google.common.cache.CacheLoader; /** * * @author Adrian Cole */ @Singleton -public class CreateUniqueKeyPair extends CacheLoader { +public class CreateUniqueKeyPair implements Function { @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; @@ -54,7 +54,7 @@ public class CreateUniqueKeyPair extends CacheLoader { } @Override - public KeyPair load(RegionAndName from) { + public KeyPair apply(RegionAndName from) { return createNewKeyPairInRegion(from.getRegion(), from.getName()); } @@ -67,7 +67,7 @@ public class CreateUniqueKeyPair extends CacheLoader { while (keyPair == null) { try { keyPair = ec2Client.getKeyPairServices().createKeyPairInRegion(region, getNextName(region, group)); - logger.debug("<< created keyPair(%s) fingerprint(%s)", keyPair.getKeyName(), keyPair.getKeyFingerprint()); + logger.debug("<< created keyPair(%s)", keyPair); } catch (IllegalStateException e) { } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CredentialsForInstance.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CredentialsForInstance.java index 11283a0a9f..00cf49b48e 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CredentialsForInstance.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/functions/CredentialsForInstance.java @@ -20,6 +20,7 @@ package org.jclouds.ec2.compute.functions; import static com.google.common.base.Preconditions.checkNotNull; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutionException; import javax.inject.Inject; @@ -43,12 +44,12 @@ import com.google.common.cache.CacheLoader; */ @Singleton public class CredentialsForInstance extends CacheLoader { - private final Cache credentialsMap; + private final ConcurrentMap credentialsMap; private final PopulateDefaultLoginCredentialsForImageStrategy credentialProvider; private final Supplier> imageMap; @Inject - CredentialsForInstance(Cache credentialsMap, + CredentialsForInstance(ConcurrentMap credentialsMap, PopulateDefaultLoginCredentialsForImageStrategy credentialProvider, Supplier> imageMap) { this.credentialsMap = checkNotNull(credentialsMap, "credentialsMap"); this.credentialProvider = checkNotNull(credentialProvider, "credentialProvider"); diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java b/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java index 15d21a2bb0..ff08613a38 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions.java @@ -18,10 +18,14 @@ */ package org.jclouds.ec2.compute.strategy; +import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static org.jclouds.crypto.SshKeys.fingerprintPrivateKey; +import static org.jclouds.crypto.SshKeys.sha1PrivateKey; import java.util.Set; +import java.util.concurrent.ConcurrentMap; import javax.inject.Inject; import javax.inject.Named; @@ -30,7 +34,6 @@ import javax.inject.Singleton; import org.jclouds.compute.domain.Template; import org.jclouds.compute.options.TemplateOptions; -import static org.jclouds.crypto.SshKeys.*; import org.jclouds.ec2.compute.domain.RegionAndName; import org.jclouds.ec2.compute.domain.RegionNameAndIngressRules; import org.jclouds.ec2.compute.options.EC2TemplateOptions; @@ -40,10 +43,10 @@ import org.jclouds.ec2.options.RunInstancesOptions; import org.jclouds.javax.annotation.Nullable; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.cache.Cache; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSet.Builder; -import com.google.common.util.concurrent.UncheckedExecutionException; /** * @@ -52,15 +55,20 @@ import com.google.common.util.concurrent.UncheckedExecutionException; @Singleton public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { @VisibleForTesting - public final Cache credentialsMap; + public Function makeKeyPair; + @VisibleForTesting + public final ConcurrentMap credentialsMap; @VisibleForTesting public final Cache securityGroupMap; - protected final Provider optionsProvider; + @VisibleForTesting + public final Provider optionsProvider; @Inject - public CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(Cache credentialsMap, + public CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(Function makeKeyPair, + ConcurrentMap credentialsMap, @Named("SECURITY") Cache securityGroupMap, Provider optionsProvider) { + this.makeKeyPair = checkNotNull(makeKeyPair, "makeKeyPair"); this.credentialsMap = checkNotNull(credentialsMap, "credentialsMap"); this.securityGroupMap = checkNotNull(securityGroupMap, "securityGroupMap"); this.optionsProvider = checkNotNull(optionsProvider, "optionsProvider"); @@ -119,31 +127,36 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { KeyPair keyPair = KeyPair.builder().region(region).keyName(keyPairName).fingerprint( fingerprintPrivateKey(pem)).sha1OfPrivateKey(sha1PrivateKey(pem)).keyMaterial(pem).build(); RegionAndName key = new RegionAndName(region, keyPairName); - credentialsMap.asMap().put(key, keyPair); + credentialsMap.put(key, keyPair); } } if (options.getRunScript() != null) { RegionAndName regionAndName = new RegionAndName(region, keyPairName); - String message = String - .format( - "no private key configured for: %s; please use options.overrideLoginCredentialWith(rsa_private_text)", - regionAndName); - // test to see if this is in cache. - try { - credentialsMap.getUnchecked(regionAndName); - } catch (NullPointerException nex) { - throw new IllegalArgumentException(message, nex); - } catch (UncheckedExecutionException nex) { - throw new IllegalArgumentException(message, nex); - } + checkArgument( + credentialsMap.containsKey(regionAndName), + "no private key configured for: %s; please use options.overrideLoginCredentialWith(rsa_private_text)", + regionAndName); } return keyPairName; } // base EC2 driver currently does not support key import protected String createOrImportKeyPair(String region, String group, TemplateOptions options) { - return credentialsMap.getUnchecked(new RegionAndName(region, group)).getKeyName(); + RegionAndName regionAndGroup = new RegionAndName(region, group); + KeyPair keyPair; + // make sure that we don't request multiple keys simultaneously + synchronized (credentialsMap) { + // if there is already a keypair for the group specified, use it + if (credentialsMap.containsKey(regionAndGroup)) + return credentialsMap.get(regionAndGroup).getKeyName(); + + // otherwise create a new keypair and key it under the group and also the regular keyname + keyPair = makeKeyPair.apply(new RegionAndName(region, group)); + credentialsMap.put(regionAndGroup, keyPair); + } + credentialsMap.put(new RegionAndName(region, keyPair.getKeyName()), keyPair); + return keyPair.getKeyName(); } @VisibleForTesting diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java index 502491bc43..6ee15cc0b4 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/compute/strategy/CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java @@ -47,6 +47,7 @@ import org.jclouds.encryption.internal.Base64; import org.jclouds.scriptbuilder.domain.Statements; import org.testng.annotations.Test; +import com.google.common.base.Function; import com.google.common.cache.Cache; import com.google.common.collect.ImmutableMultimap; import com.google.common.collect.ImmutableSet; @@ -251,8 +252,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { expect(options.getOverridingCredentials()).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andThrow( - new NullPointerException()); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(false); // replay mocks replay(options); @@ -284,7 +284,7 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { expect(options.getOverridingCredentials()).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andReturn(keyPair); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(true); // replay mocks replay(options); @@ -300,7 +300,6 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { verifyStrategy(strategy); } - @SuppressWarnings("unchecked") public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_reusesKeyWhenToldToWithRunScriptAndCredentialsSpecified() { // setup constants String region = Region.AP_SOUTHEAST_1; @@ -311,22 +310,19 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions strategy = setupStrategy(); EC2TemplateOptions options = createMock(EC2TemplateOptions.class); KeyPair keyPair = createMock(KeyPair.class); - ConcurrentMap backing = createMock(ConcurrentMap.class); // setup expectations expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.getOverridingCredentials()).andReturn(CREDENTIALS).atLeastOnce(); - expect(strategy.credentialsMap.asMap()).andReturn(backing); // Notice that the fingerprint and sha1 generated - expect(backing.put(new RegionAndName(region, userSuppliedKeyPair), KEYPAIR)).andReturn(null); + expect(strategy.credentialsMap.put(new RegionAndName(region, userSuppliedKeyPair), KEYPAIR)).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andReturn(keyPair); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(true); // replay mocks replay(options); replay(keyPair); - replay(backing); replayStrategy(strategy); // run @@ -335,7 +331,6 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { // verify mocks verify(options); verify(keyPair); - verify(backing); verifyStrategy(strategy); } @@ -357,7 +352,8 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.shouldAutomaticallyCreateKeyPair()).andReturn(shouldAutomaticallyCreateKeyPair); expect(keyPair.getKeyName()).andReturn(systemGeneratedKeyPairName).atLeastOnce(); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, group))).andReturn(keyPair); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, group))).andReturn(true); + expect(strategy.credentialsMap.get(new RegionAndName(region, group))).andReturn(keyPair); expect(options.getRunScript()).andReturn(null); // replay mocks @@ -545,19 +541,22 @@ public class CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptionsTest { } private void verifyStrategy(CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions strategy) { + verify(strategy.makeKeyPair); verify(strategy.credentialsMap); verify(strategy.securityGroupMap); } @SuppressWarnings("unchecked") private CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions setupStrategy() { - Cache credentialsMap = createMock(Cache.class); + Function makeKeyPair = createMock(Function.class); + ConcurrentMap credentialsMap = createMock(ConcurrentMap.class); Cache securityGroupMap = createMock(Cache.class); - return new CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(credentialsMap, securityGroupMap, - OPTIONS_PROVIDER); + return new CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap, + securityGroupMap, OPTIONS_PROVIDER); } private void replayStrategy(CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions strategy) { + replay(strategy.makeKeyPair); replay(strategy.credentialsMap); replay(strategy.securityGroupMap); } diff --git a/core/src/main/java/org/jclouds/crypto/SshKeys.java b/core/src/main/java/org/jclouds/crypto/SshKeys.java index 03ab33c9e7..f901b97f4f 100644 --- a/core/src/main/java/org/jclouds/crypto/SshKeys.java +++ b/core/src/main/java/org/jclouds/crypto/SshKeys.java @@ -232,6 +232,16 @@ public class SshKeys { return fingerprint(certKeySpec.getPublicExponent(), certKeySpec.getModulus()); } + /** + * @param publicKeyOpenSSH + * RSA public key in OpenSSH format + * @return fingerprint ex. {@code 2b:a9:62:95:5b:8b:1d:61:e0:92:f7:03:10:e9:db:d9} + */ + public static String fingerprintPublicKey(String publicKeyOpenSSH) { + RSAPublicKeySpec publicKeySpec = publicKeySpecFromOpenSSH(publicKeyOpenSSH); + return fingerprint(publicKeySpec.getPublicExponent(), publicKeySpec.getModulus()); + } + /** * @return true if the keypair has the same SHA1 fingerprint as supplied */ diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java index 16848197c3..3c397d4b60 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/AWSEC2ComputeService.java @@ -22,8 +22,9 @@ import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.Iterables.transform; import java.util.Map; -import java.util.Map.Entry; import java.util.Set; +import java.util.Map.Entry; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.ExecutorService; import javax.inject.Inject; @@ -99,7 +100,7 @@ public class AWSEC2ComputeService extends EC2ComputeService { RunScriptOnNode.Factory runScriptOnNodeFactory, InitAdminAccess initAdminAccess, PersistNodeCredentials persistNodeCredentials, Timeouts timeouts, @Named(Constants.PROPERTY_USER_THREADS) ExecutorService executor, AWSEC2Client ec2Client, - Cache credentialsMap, + ConcurrentMap credentialsMap, @Named("SECURITY") Cache securityGroupMap, @Named("PLACEMENT") Cache placementGroupMap, @Named("DELETED") Predicate placementGroupDeleted) { diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceDependenciesModule.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceDependenciesModule.java index 2ee7fc70e2..4007ba24b4 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceDependenciesModule.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/config/AWSEC2ComputeServiceDependenciesModule.java @@ -84,7 +84,7 @@ public class AWSEC2ComputeServiceDependenciesModule extends EC2ComputeServiceDep }).to(CredentialsForInstance.class); bind(new TypeLiteral>() { }).to(CreateSecurityGroupIfNeeded.class); - bind(new TypeLiteral>() { + bind(new TypeLiteral>() { }).to(CreateUniqueKeyPair.class); bind(new TypeLiteral>() { }).to(ImportOrReturnExistingKeypair.class); diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java index 679af119bb..4817bb3453 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions.java @@ -21,6 +21,8 @@ package org.jclouds.aws.ec2.compute.strategy; import static com.google.common.base.Predicates.and; import static com.google.common.base.Predicates.or; +import java.util.concurrent.ConcurrentMap; + import javax.annotation.Resource; import javax.inject.Inject; import javax.inject.Named; @@ -52,7 +54,7 @@ import com.google.common.cache.Cache; */ @Singleton public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions extends - CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { + CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions { @Resource @Named(ComputeServiceConstants.COMPUTE_LOGGER) protected Logger logger = Logger.NULL; @@ -65,13 +67,13 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions @Inject public CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions( - Cache credentialsMap, + Function makeKeyPair, ConcurrentMap credentialsMap, @Named("SECURITY") Cache securityGroupMap, Provider optionsProvider, @Named("PLACEMENT") Cache placementGroupMap, CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded, Function importExistingKeyPair) { - super(credentialsMap, securityGroupMap, optionsProvider); + super(makeKeyPair, credentialsMap, securityGroupMap, optionsProvider); this.placementGroupMap = placementGroupMap; this.createPlacementGroupIfNeeded = createPlacementGroupIfNeeded; this.importExistingKeyPair = importExistingKeyPair; @@ -79,10 +81,11 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions public AWSRunInstancesOptions execute(String region, String group, Template template) { AWSRunInstancesOptions instanceOptions = AWSRunInstancesOptions.class - .cast(super.execute(region, group, template)); + .cast(super.execute(region, group, template)); String placementGroupName = template.getHardware().getId().startsWith("cc") ? createNewPlacementGroupUnlessUserSpecifiedOtherwise( - region, group, template.getOptions()) : null; + region, group, template.getOptions()) + : null; if (placementGroupName != null) instanceOptions.inPlacementGroup(placementGroupName); @@ -101,7 +104,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions placementGroupName = AWSEC2TemplateOptions.class.cast(options).getPlacementGroup(); if (placementGroupName == null) shouldAutomaticallyCreatePlacementGroup = AWSEC2TemplateOptions.class.cast(options) - .shouldAutomaticallyCreatePlacementGroup(); + .shouldAutomaticallyCreatePlacementGroup(); } if (placementGroupName == null && shouldAutomaticallyCreatePlacementGroup) { // placementGroupName must be unique within an account per @@ -118,17 +121,16 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions public String createNewKeyPairUnlessUserSpecifiedOtherwise(String region, String group, TemplateOptions options) { RegionAndName key = new RegionAndName(region, group); KeyPair pair; - if (credentialsMap.asMap().containsKey(key)) - return credentialsMap.getUnchecked(key).getKeyName(); if (and(hasPublicKeyMaterial, or(doesntNeedSshAfterImportingPublicKey, hasLoginCredential)).apply(options)) { pair = importExistingKeyPair.apply(new RegionNameAndPublicKeyMaterial(region, group, options.getPublicKey())); options.dontAuthorizePublicKey(); if (hasLoginCredential.apply(options)) pair = pair.toBuilder().keyMaterial(options.getOverridingCredentials().credential).build(); - credentialsMap.asMap().put(key, pair); + credentialsMap.put(key, pair); } else { if (hasPublicKeyMaterial.apply(options)) { - logger.warn("to avoid creating temporary keys in aws-ec2, use templateOption overrideLoginCredentialWith(id_rsa)"); + logger + .warn("to avoid creating temporary keys in aws-ec2, use templateOption overrideLoginCredentialWith(id_rsa)"); } return super.createNewKeyPairUnlessUserSpecifiedOtherwise(region, group, options); } @@ -165,8 +167,8 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions @Override protected boolean userSpecifiedTheirOwnGroups(TemplateOptions options) { return options instanceof AWSEC2TemplateOptions - && AWSEC2TemplateOptions.class.cast(options).getGroupIds().size() > 0 - || super.userSpecifiedTheirOwnGroups(options); + && AWSEC2TemplateOptions.class.cast(options).getGroupIds().size() > 0 + || super.userSpecifiedTheirOwnGroups(options); } @Override diff --git a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypair.java b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypair.java index 519d2e819f..4474543ce7 100644 --- a/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypair.java +++ b/providers/aws-ec2/src/main/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypair.java @@ -19,6 +19,7 @@ package org.jclouds.aws.ec2.functions; import static com.google.common.base.Preconditions.checkNotNull; +import static org.jclouds.crypto.SshKeys.fingerprintPublicKey; import javax.annotation.Resource; import javax.inject.Inject; @@ -68,15 +69,24 @@ public class ImportOrReturnExistingKeypair implements Function newConcurrentMap()); expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.getPublicKey()).andReturn(null).times(2); expect(options.getOverridingCredentials()).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andThrow( - new NullPointerException()); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(false); // replay mocks replay(options); @@ -433,12 +430,11 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT KeyPair keyPair = createMock(KeyPair.class); // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(Maps. newConcurrentMap()); expect(options.getPublicKey()).andReturn(null).times(2); expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.getOverridingCredentials()).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andReturn(keyPair); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(true); // replay mocks replay(options); @@ -454,8 +450,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT verifyStrategy(strategy); } - @SuppressWarnings("unchecked") - public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_reusesKeyWhenToldToWithRunScriptAndCredentialsSpecified() { + public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_ImportPublicKeyWhenOptionIsSet() { // setup constants String region = Region.AP_SOUTHEAST_1; String group = "group"; @@ -466,22 +461,20 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT EC2TemplateOptions options = createMock(EC2TemplateOptions.class); KeyPair keyPair = createMock(KeyPair.class); - ConcurrentMap backing = createMock(ConcurrentMap.class); - - // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(backing).atLeastOnce(); - expect(backing.containsKey(new RegionAndName(region, group))).andReturn(false); - expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); - expect(options.getPublicKey()).andReturn(null).times(2); + // we specify we have a public key we want to use for authentication + expect(options.getPublicKey()).andReturn("ssh-rsa").times(2); expect(options.getOverridingCredentials()).andReturn(CREDENTIALS).atLeastOnce(); - expect(backing.put(new RegionAndName(region, userSuppliedKeyPair), KEYPAIR)).andReturn(null); + + // Here, we import the keypair and place it into the cache + expect(strategy.importExistingKeyPair.apply(new RegionNameAndPublicKeyMaterial(region, group, "ssh-rsa"))) + .andReturn(KEYPAIR); + expect(options.dontAuthorizePublicKey()).andReturn(options); + expect(strategy.credentialsMap.put(new RegionAndName(region, group), KEYPAIR)).andReturn(null); expect(options.getRunScript()).andReturn(Statements.exec("echo foo")); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, userSuppliedKeyPair))).andReturn(keyPair); // replay mocks replay(options); replay(keyPair); - replay(backing); replayStrategy(strategy); // run @@ -490,11 +483,9 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // verify mocks verify(options); verify(keyPair); - verify(backing); verifyStrategy(strategy); } - @SuppressWarnings("unchecked") @Test public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_importsKeyPairAndUnsetsTemplateInstructionWhenPublicKeySuppliedAndAddsCredentialToMapWhenOverridingCredsAreSet() { // setup constants @@ -506,31 +497,25 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT AWSEC2TemplateOptions options = keyPair(group).authorizePublicKey("ssh-rsa").overrideCredentialsWith( new Credentials("foo", CREDENTIALS.credential)); KeyPair keyPair = new KeyPair(region, group, "//TODO", null, null); - ConcurrentMap backing = createMock(ConcurrentMap.class); // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(backing).atLeastOnce(); - expect(backing.containsKey(new RegionAndName(region, group))).andReturn(false); expect( strategy.importExistingKeyPair.apply(new RegionNameAndPublicKeyMaterial(region, group, CREDENTIALS.credential))).andReturn(keyPair); expect( - backing.put(new RegionAndName(region, group), keyPair.toBuilder().keyMaterial(CREDENTIALS.credential) - .build())).andReturn(null); + strategy.credentialsMap.put(new RegionAndName(region, group), keyPair.toBuilder().keyMaterial( + CREDENTIALS.credential).build())).andReturn(null); // replay mocks - replay(backing); replayStrategy(strategy); // run assertEquals(strategy.createNewKeyPairUnlessUserSpecifiedOtherwise(region, group, options), group); // verify mocks - verify(backing); verifyStrategy(strategy); } - @SuppressWarnings("unchecked") @Test public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_importsKeyPairAndUnsetsTemplateInstructionWhenPublicKeySupplied() { // setup constants @@ -543,24 +528,18 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT KeyPair keyPair = new KeyPair(region, "jclouds#" + group, "fingerprint", null, null); - ConcurrentMap backing = createMock(ConcurrentMap.class); - // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(backing).atLeastOnce(); - expect(backing.containsKey(new RegionAndName(region, group))).andReturn(false); expect(strategy.importExistingKeyPair.apply(new RegionNameAndPublicKeyMaterial(region, group, "ssh-rsa"))) .andReturn(keyPair); - expect(backing.put(new RegionAndName(region, group), keyPair)).andReturn(null); + expect(strategy.credentialsMap.put(new RegionAndName(region, group), keyPair)).andReturn(null); // replay mocks - replay(backing); replayStrategy(strategy); // run assertEquals(strategy.createNewKeyPairUnlessUserSpecifiedOtherwise(region, group, options), "jclouds#" + group); // verify mocks - verify(backing); verifyStrategy(strategy); } @@ -578,10 +557,10 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT KeyPair keyPair = createMock(KeyPair.class); // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(Maps. newConcurrentMap()); expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.shouldAutomaticallyCreateKeyPair()).andReturn(shouldAutomaticallyCreateKeyPair); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, group))).andReturn(keyPair); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, group))).andReturn(true); + expect(strategy.credentialsMap.get(new RegionAndName(region, group))).andReturn(keyPair); expect(options.getPublicKey()).andReturn(null).times(2); expect(keyPair.getKeyName()).andReturn(systemGeneratedKeyPairName).atLeastOnce(); expect(options.getRunScript()).andReturn(null); @@ -601,7 +580,6 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT verifyStrategy(strategy); } - @SuppressWarnings("unchecked") public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_returnsExistingKeyIfAlreadyPresent() { // setup constants String region = Region.AP_SOUTHEAST_1; @@ -612,18 +590,18 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions strategy = setupStrategy(); AWSEC2TemplateOptions options = createMock(AWSEC2TemplateOptions.class); KeyPair keyPair = createMock(KeyPair.class); - ConcurrentMap backing = createMock(ConcurrentMap.class); // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(backing); - expect(backing.containsKey(new RegionAndName(region, group))).andReturn(true); - expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, group))).andReturn(keyPair); - expect(keyPair.getKeyName()).andReturn(systemGeneratedKeyPairName).atLeastOnce(); + expect(options.getOverridingCredentials()).andReturn(null); + expect(options.getRunScript()).andReturn(Statements.exec("echo hello")); + expect(options.getPublicKey()).andReturn(null).times(2); + expect(options.getKeyPair()).andReturn(systemGeneratedKeyPairName); + expect(strategy.credentialsMap.containsKey(new RegionAndName(region, systemGeneratedKeyPairName))) + .andReturn(true); // replay mocks replay(options); replay(keyPair); - replay(backing); replayStrategy(strategy); // run @@ -633,7 +611,6 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT // verify mocks verify(options); verify(keyPair); - verify(backing); verifyStrategy(strategy); } @@ -651,7 +628,6 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT KeyPair keyPair = createMock(KeyPair.class); // setup expectations - expect(strategy.credentialsMap.asMap()).andReturn(Maps. newConcurrentMap()); expect(options.getPublicKey()).andReturn(null).times(2); expect(options.getKeyPair()).andReturn(userSuppliedKeyPair); expect(options.getRunScript()).andReturn(null); @@ -937,6 +913,7 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT } private void verifyStrategy(CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions strategy) { + verify(strategy.makeKeyPair); verify(strategy.credentialsMap); verify(strategy.securityGroupMap); verify(strategy.placementGroupMap); @@ -946,17 +923,20 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT @SuppressWarnings("unchecked") private CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions setupStrategy() { - Cache credentialsMap = createMock(Cache.class); + Function makeKeyPair = createMock(Function.class); + ConcurrentMap credentialsMap = createMock(ConcurrentMap.class); Cache securityGroupMap = createMock(Cache.class); Cache placementGroupMap = createMock(Cache.class); Function importExistingKeyPair = createMock(Function.class); CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded = createMock(CreatePlacementGroupIfNeeded.class); - return new CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions(credentialsMap, securityGroupMap, - OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded, importExistingKeyPair); + return new CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap, + securityGroupMap, OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded, + importExistingKeyPair); } private void replayStrategy(CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions strategy) { + replay(strategy.makeKeyPair); replay(strategy.credentialsMap); replay(strategy.securityGroupMap); replay(strategy.placementGroupMap); diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypairTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypairTest.java index 4fe2625452..e1c0a082cd 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypairTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/functions/ImportOrReturnExistingKeypairTest.java @@ -22,10 +22,9 @@ import static org.easymock.EasyMock.expect; import static org.easymock.classextension.EasyMock.createMock; import static org.easymock.classextension.EasyMock.replay; import static org.easymock.classextension.EasyMock.verify; +import static org.jclouds.crypto.SshKeys.fingerprintPublicKey; import static org.testng.Assert.assertEquals; -import java.net.UnknownHostException; - import org.jclouds.aws.ec2.AWSEC2Client; import org.jclouds.aws.ec2.services.AWSKeyPairClient; import org.jclouds.ec2.domain.KeyPair; @@ -38,39 +37,42 @@ import com.google.common.collect.ImmutableSet; */ @Test(groups = "unit") public class ImportOrReturnExistingKeypairTest { + private static final String PUBLIC_KEY = "ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQCcm8DjTHg4r72dVhNLQ33XpUyMLr+ph78i4NR3LqF1bXDP0g4xNLcI/GUTQq6g07X8zs7vIWyjoitqBPFSQ2onaZQ6pXQF/QISRQgrN5hEZ+nH5Aw+isdstBeOMWKdYrCJtm6/qWq2+rByyuNbtulazP3H7SqoozSjRSGNQyFNGpmhjGgTbNQftYDwlFq0T9tCSO/+dYF8j79bNIOEmfsCMiqXQ13hD5vGiEgkRm7zIPDUfpOl3ubDzebpRgGTh5kfv2vd3Z665AxQoi6fItvDu80knyphMlC41giIm5YqfPOPG4lR+6aF06p+NKhvOeECNMtRsD9u1kKJD9NqxXhx"; + + private static final KeyPair pair = KeyPair.builder().region("region").keyName("jclouds#group").sha1OfPrivateKey( + "foo").build(); + private static final KeyPair pairWithFingerprint = pair.toBuilder().fingerprint(fingerprintPublicKey(PUBLIC_KEY)) + .build(); + @Test - public void testApply() throws UnknownHostException { + public void testApply() { AWSEC2Client client = createMock(AWSEC2Client.class); AWSKeyPairClient keyClient = createMock(AWSKeyPairClient.class); - KeyPair pair = createMock(KeyPair.class); - expect(client.getKeyPairServices()).andReturn(keyClient).atLeastOnce(); - expect(keyClient.importKeyPairInRegion("region", "jclouds#group", "ssh-rsa")).andReturn(pair); + expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andReturn(pair); replay(client); replay(keyClient); ImportOrReturnExistingKeypair parser = new ImportOrReturnExistingKeypair(client); - assertEquals(parser.importOrReturnExistingKeypair("region", "group", "ssh-rsa"), pair); + assertEquals(parser.importOrReturnExistingKeypair("region", "group", PUBLIC_KEY), pairWithFingerprint); verify(client); verify(keyClient); } @Test - public void testApplyWithIllegalStateExceptionReturnsExistingKey() throws UnknownHostException { + public void testApplyWithIllegalStateExceptionReturnsExistingKey() { AWSEC2Client client = createMock(AWSEC2Client.class); AWSKeyPairClient keyClient = createMock(AWSKeyPairClient.class); - KeyPair pair = createMock(KeyPair.class); - expect(client.getKeyPairServices()).andReturn(keyClient).atLeastOnce(); - expect(keyClient.importKeyPairInRegion("region", "jclouds#group", "ssh-rsa")).andThrow( - new IllegalStateException()); + expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow( + new IllegalStateException()); expect(keyClient.describeKeyPairsInRegion("region", "jclouds#group")).andReturn(ImmutableSet.of(pair)); replay(client); @@ -78,7 +80,8 @@ public class ImportOrReturnExistingKeypairTest { ImportOrReturnExistingKeypair parser = new ImportOrReturnExistingKeypair(client); - assertEquals(parser.importOrReturnExistingKeypair("region", "group", "ssh-rsa"), pair); + // enriching to include the ssh fingerprint so that ssh logs are easier to correlate + assertEquals(parser.importOrReturnExistingKeypair("region", "group", PUBLIC_KEY), pairWithFingerprint); verify(client); verify(keyClient); @@ -86,19 +89,17 @@ public class ImportOrReturnExistingKeypairTest { } @Test - public void testApplyWithIllegalStateExceptionRetriesWhenExistingKeyNotFound() throws UnknownHostException { + public void testApplyWithIllegalStateExceptionRetriesWhenExistingKeyNotFound() { AWSEC2Client client = createMock(AWSEC2Client.class); AWSKeyPairClient keyClient = createMock(AWSKeyPairClient.class); - KeyPair pair = createMock(KeyPair.class); - expect(client.getKeyPairServices()).andReturn(keyClient).atLeastOnce(); - expect(keyClient.importKeyPairInRegion("region", "jclouds#group", "ssh-rsa")).andThrow( - new IllegalStateException()); + expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow( + new IllegalStateException()); expect(keyClient.describeKeyPairsInRegion("region", "jclouds#group")).andReturn(ImmutableSet. of()); - expect(keyClient.importKeyPairInRegion("region", "jclouds#group", "ssh-rsa")).andThrow( - new IllegalStateException()); + expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow( + new IllegalStateException()); expect(keyClient.describeKeyPairsInRegion("region", "jclouds#group")).andReturn(ImmutableSet. of(pair)); replay(client); @@ -106,7 +107,7 @@ public class ImportOrReturnExistingKeypairTest { ImportOrReturnExistingKeypair parser = new ImportOrReturnExistingKeypair(client); - assertEquals(parser.importOrReturnExistingKeypair("region", "group", "ssh-rsa"), pair); + assertEquals(parser.importOrReturnExistingKeypair("region", "group", PUBLIC_KEY), pairWithFingerprint); verify(client); verify(keyClient); diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AWSKeyPairClientLiveTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AWSKeyPairClientLiveTest.java index a65b6a5b62..15c3a81d71 100644 --- a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AWSKeyPairClientLiveTest.java +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AWSKeyPairClientLiveTest.java @@ -130,10 +130,11 @@ public class AWSKeyPairClientLiveTest { Set nodes = noSshContext.getComputeService().createNodesInGroup(group, 1, options); NodeMetadata first = get(nodes, 0); - // credentials should be present as this is the default user from the image assert first.getCredentials() != null : first; assert first.getCredentials().identity != null : first; - assert first.getCredentials().credential != null : first; + // credentials should not be present as the import public key call doesn't have access to + // the related private key + assert first.getCredentials().credential == null : first; AWSRunningInstance instance = getInstance(instanceClient, first.getProviderId());