Issue 717: corrected keyPair state logic which was redundantly creating keys

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<RegionAndName, KeyPair> credentialsMap;
+   private final ConcurrentMap<RegionAndName, KeyPair> credentialsMap;
    private final Cache<RegionAndName, String> 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<RegionAndName, KeyPair> credentialsMap, @Named("SECURITY") Cache<RegionAndName, String> securityGroupMap) {
+         ConcurrentMap<RegionAndName, KeyPair> credentialsMap, @Named("SECURITY") Cache<RegionAndName, String> 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());
             }
          }

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<CacheLoader<RegionAndName, String>>() {
       }).to(CreateSecurityGroupIfNeeded.class);
-      bind(new TypeLiteral<CacheLoader<RegionAndName, KeyPair>>() {
+      bind(new TypeLiteral<Function<RegionAndName, KeyPair>>() {
       }).to(CreateUniqueKeyPair.class);
       bind(new TypeLiteral<CacheLoader<RegionAndName, Image>>() {
       }).to(RegionAndIdToImage.class);
@@ -131,8 +134,8 @@ public class EC2ComputeServiceDependenciesModule extends AbstractModule {
 
    @Provides
    @Singleton
-   protected Cache<RegionAndName, KeyPair> keypairMap(CacheLoader<RegionAndName, KeyPair> in) {
+   protected ConcurrentMap<RegionAndName, KeyPair> keypairMap(Injector i) {
-      return CacheBuilder.newBuilder().build(in);
+      return Maps.newConcurrentMap();
    }
    
    @Provides

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<RegionAndName, KeyPair> {
+public class CreateUniqueKeyPair implements Function<RegionAndName, KeyPair> {
    @Resource
    @Named(ComputeServiceConstants.COMPUTE_LOGGER)
    protected Logger logger = Logger.NULL;
@@ -54,7 +54,7 @@ public class CreateUniqueKeyPair extends CacheLoader<RegionAndName, KeyPair> {
    }
 
    @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<RegionAndName, KeyPair> {
       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) {
 
          }

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<RunningInstance, Credentials> {
-   private final Cache<RegionAndName, KeyPair> credentialsMap;
+   private final ConcurrentMap<RegionAndName, KeyPair> credentialsMap;
    private final PopulateDefaultLoginCredentialsForImageStrategy credentialProvider;
    private final Supplier<Cache<RegionAndName, ? extends Image>> imageMap;
 
    @Inject
-   CredentialsForInstance(Cache<RegionAndName, KeyPair> credentialsMap,
+   CredentialsForInstance(ConcurrentMap<RegionAndName, KeyPair> credentialsMap,
          PopulateDefaultLoginCredentialsForImageStrategy credentialProvider, Supplier<Cache<RegionAndName, ? extends Image>> imageMap) {
       this.credentialsMap = checkNotNull(credentialsMap, "credentialsMap");
       this.credentialProvider = checkNotNull(credentialProvider, "credentialProvider");

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<RegionAndName, KeyPair> credentialsMap;
+   public Function<RegionAndName, KeyPair> makeKeyPair;
+   @VisibleForTesting
+   public final ConcurrentMap<RegionAndName, KeyPair> credentialsMap;
    @VisibleForTesting
    public final Cache<RegionAndName, String> securityGroupMap;
-   protected final Provider<RunInstancesOptions> optionsProvider;
+   @VisibleForTesting
+   public final Provider<RunInstancesOptions> optionsProvider;
 
    @Inject
-   public CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(Cache<RegionAndName, KeyPair> credentialsMap,
+   public CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(Function<RegionAndName, KeyPair> makeKeyPair,
+            ConcurrentMap<RegionAndName, KeyPair> credentialsMap,
             @Named("SECURITY") Cache<RegionAndName, String> securityGroupMap,
             Provider<RunInstancesOptions> 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
+         checkArgument(
-                  .format(
+                  credentialsMap.containsKey(regionAndName),
-                           "no private key configured for: %s; please use options.overrideLoginCredentialWith(rsa_private_text)",
+                  "no private key configured for: %s; please use options.overrideLoginCredentialWith(rsa_private_text)",
-                           regionAndName);
+                  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);
-         }
       }
       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

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(
+      expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(false);
-               new NullPointerException());
 
       // 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<RegionAndName, KeyPair> 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<RegionAndName, KeyPair> credentialsMap = createMock(Cache.class);
+      Function<RegionAndName, KeyPair> makeKeyPair = createMock(Function.class);
+      ConcurrentMap<RegionAndName, KeyPair> credentialsMap = createMock(ConcurrentMap.class);
       Cache<RegionAndName, String> securityGroupMap = createMock(Cache.class);
-      return new CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(credentialsMap, securityGroupMap,
+      return new CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap,
-               OPTIONS_PROVIDER);
+               securityGroupMap, OPTIONS_PROVIDER);
    }
 
    private void replayStrategy(CreateKeyPairAndSecurityGroupsAsNeededAndReturnRunOptions strategy) {
+      replay(strategy.makeKeyPair);
       replay(strategy.credentialsMap);
       replay(strategy.securityGroupMap);
    }

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
     */

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<RegionAndName, KeyPair> credentialsMap,
+         ConcurrentMap<RegionAndName, KeyPair> credentialsMap,
          @Named("SECURITY") Cache<RegionAndName, String> securityGroupMap,
          @Named("PLACEMENT") Cache<RegionAndName, String> placementGroupMap,
          @Named("DELETED") Predicate<PlacementGroup> placementGroupDeleted) {

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<CacheLoader<RegionAndName, String>>() {
       }).to(CreateSecurityGroupIfNeeded.class);
-      bind(new TypeLiteral<CacheLoader<RegionAndName, KeyPair>>() {
+      bind(new TypeLiteral<Function<RegionAndName, KeyPair>>() {
       }).to(CreateUniqueKeyPair.class);
       bind(new TypeLiteral<Function<RegionNameAndPublicKeyMaterial, KeyPair>>() {
       }).to(ImportOrReturnExistingKeypair.class);

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<RegionAndName, KeyPair> credentialsMap,
+            Function<RegionAndName, KeyPair> makeKeyPair, ConcurrentMap<RegionAndName, KeyPair> credentialsMap,
             @Named("SECURITY") Cache<RegionAndName, String> securityGroupMap,
             Provider<RunInstancesOptions> optionsProvider,
             @Named("PLACEMENT") Cache<RegionAndName, String> placementGroupMap,
             CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded,
             Function<RegionNameAndPublicKeyMaterial, KeyPair> 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
+               && AWSEC2TemplateOptions.class.cast(options).getGroupIds().size() > 0
-            || super.userSpecifiedTheirOwnGroups(options);
+               || super.userSpecifiedTheirOwnGroups(options);
    }
 
    @Override

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<RegionNameAndPubl
       while (keyPair == null)
          try {
             keyPair = ec2Client.getKeyPairServices().importKeyPairInRegion(region, "jclouds#" + group,
-                  publicKeyMaterial);
+                     publicKeyMaterial);
+            keyPair = addFingerprintToKeyPair(publicKeyMaterial, keyPair);
             logger.debug("<< imported keyPair(%s)", keyPair);
          } catch (IllegalStateException e) {
-            keyPair = Iterables.getFirst(
+            keyPair = Iterables.getFirst(ec2Client.getKeyPairServices().describeKeyPairsInRegion(region,
-                  ec2Client.getKeyPairServices().describeKeyPairsInRegion(region, "jclouds#" + group), null);
+                     "jclouds#" + group), null);
-            if (keyPair != null)
+            if (keyPair != null) {
+               keyPair = addFingerprintToKeyPair(publicKeyMaterial, keyPair);
                logger.debug("<< retrieved existing keyPair(%s)", keyPair);
+            }
          }
       return keyPair;
    }
 
+   public KeyPair addFingerprintToKeyPair(String publicKeyMaterial, KeyPair keyPair) {
+      // add in the fingerprint as it makes correlating keys in ssh logs possible
+      keyPair = keyPair.toBuilder().fingerprint(fingerprintPublicKey(publicKeyMaterial)).build();
+      return keyPair;
+   }
+
 }

providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/compute/strategy/CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsTest.java

@@ -60,7 +60,6 @@ import com.google.common.base.Function;
 import com.google.common.cache.Cache;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
 
 /**
  * @author Adrian Cole
@@ -399,13 +398,11 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT
       KeyPair keyPair = createMock(KeyPair.class);
 
       // setup expectations
-      expect(strategy.credentialsMap.asMap()).andReturn(Maps.<RegionAndName, KeyPair> 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(
+      expect(strategy.credentialsMap.containsKey(new RegionAndName(region, userSuppliedKeyPair))).andReturn(false);
-               new NullPointerException());
 
       // replay mocks
       replay(options);
@@ -433,12 +430,11 @@ public class CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptionsT
       KeyPair keyPair = createMock(KeyPair.class);
 
       // setup expectations
-      expect(strategy.credentialsMap.asMap()).andReturn(Maps.<RegionAndName, KeyPair> 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_ImportPublicKeyWhenOptionIsSet() {
-   public void testCreateNewKeyPairUnlessUserSpecifiedOtherwise_reusesKeyWhenToldToWithRunScriptAndCredentialsSpecified() {
       // 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<RegionAndName, KeyPair> backing = createMock(ConcurrentMap.class);
+      // we specify we have a public key we want to use for authentication
-
+      expect(options.getPublicKey()).andReturn("ssh-rsa").times(2);
-      // 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);
       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<RegionAndName, KeyPair> 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)
+               strategy.credentialsMap.put(new RegionAndName(region, group), keyPair.toBuilder().keyMaterial(
-                        .build())).andReturn(null);
+                        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<RegionAndName, KeyPair> 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.<RegionAndName, KeyPair> 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<RegionAndName, KeyPair> backing = createMock(ConcurrentMap.class);
 
       // setup expectations
-      expect(strategy.credentialsMap.asMap()).andReturn(backing);
+      expect(options.getOverridingCredentials()).andReturn(null);
-      expect(backing.containsKey(new RegionAndName(region, group))).andReturn(true);
+      expect(options.getRunScript()).andReturn(Statements.exec("echo hello"));
-      expect(strategy.credentialsMap.getUnchecked(new RegionAndName(region, group))).andReturn(keyPair);
+      expect(options.getPublicKey()).andReturn(null).times(2);
-      expect(keyPair.getKeyName()).andReturn(systemGeneratedKeyPairName).atLeastOnce();
+      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.<RegionAndName, KeyPair> 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<RegionAndName, KeyPair> credentialsMap = createMock(Cache.class);
+      Function<RegionAndName, KeyPair> makeKeyPair = createMock(Function.class);
+      ConcurrentMap<RegionAndName, KeyPair> credentialsMap = createMock(ConcurrentMap.class);
       Cache<RegionAndName, String> securityGroupMap = createMock(Cache.class);
       Cache<RegionAndName, String> placementGroupMap = createMock(Cache.class);
       Function<RegionNameAndPublicKeyMaterial, KeyPair> importExistingKeyPair = createMock(Function.class);
       CreatePlacementGroupIfNeeded createPlacementGroupIfNeeded = createMock(CreatePlacementGroupIfNeeded.class);
 
-      return new CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions(credentialsMap, securityGroupMap,
+      return new CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions(makeKeyPair, credentialsMap,
-               OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded, importExistingKeyPair);
+               securityGroupMap, OPTIONS_PROVIDER, placementGroupMap, createPlacementGroupIfNeeded,
+               importExistingKeyPair);
    }
 
    private void replayStrategy(CreateKeyPairPlacementAndSecurityGroupsAsNeededAndReturnRunOptions strategy) {
+      replay(strategy.makeKeyPair);
       replay(strategy.credentialsMap);
       replay(strategy.securityGroupMap);
       replay(strategy.placementGroupMap);

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(
+      expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow(
-            new IllegalStateException());
+               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(
+      expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow(
-            new IllegalStateException());
+               new IllegalStateException());
       expect(keyClient.describeKeyPairsInRegion("region", "jclouds#group")).andReturn(ImmutableSet.<KeyPair> of());
-      expect(keyClient.importKeyPairInRegion("region", "jclouds#group", "ssh-rsa")).andThrow(
+      expect(keyClient.importKeyPairInRegion("region", "jclouds#group", PUBLIC_KEY)).andThrow(
-            new IllegalStateException());
+               new IllegalStateException());
       expect(keyClient.describeKeyPairsInRegion("region", "jclouds#group")).andReturn(ImmutableSet.<KeyPair> 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);

providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/services/AWSKeyPairClientLiveTest.java

@@ -130,10 +130,11 @@ public class AWSKeyPairClientLiveTest {
          Set<? extends NodeMetadata> 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());