tidy + make sure we use private key and not password for auth when keypair option set

labs/openstack-nova/src/main/java/org/jclouds/openstack/nova/v1_1/compute/NovaComputeServiceAdapter.java

@@ -19,6 +19,8 @@
 package org.jclouds.openstack.nova.v1_1.compute;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 
 import java.util.Set;
 
@@ -26,7 +28,6 @@ import javax.annotation.Resource;
 import javax.inject.Inject;
 import javax.inject.Named;
 
-import com.google.common.cache.LoadingCache;
 import org.jclouds.compute.ComputeServiceAdapter;
 import org.jclouds.compute.domain.Template;
 import org.jclouds.compute.reference.ComputeServiceConstants;
@@ -52,9 +53,10 @@ import org.jclouds.openstack.nova.v1_1.options.CreateServerOptions;
 import org.jclouds.openstack.nova.v1_1.predicates.ImagePredicates;
 
 import com.google.common.base.Function;
+import com.google.common.base.Optional;
 import com.google.common.base.Supplier;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableSet;
-import static com.google.common.collect.Iterables.*;
 import com.google.common.collect.ImmutableSet.Builder;
 
 /**
@@ -97,24 +99,34 @@ public class NovaComputeServiceAdapter implements
 
       LoginCredentials.Builder credentialsBuilder = LoginCredentials.builder();
       NovaTemplateOptions templateOptions = template.getOptions().as(NovaTemplateOptions.class);
+
       CreateServerOptions options = new CreateServerOptions();
+      options.metadata(templateOptions.getUserMetadata());
       options.securityGroupNames(templateOptions.getSecurityGroupNames());
 
-      String keyName = templateOptions.getKeyPairName();
+      Optional<String> privateKey = Optional.absent();
-      if (keyName != null) {
+      if (templateOptions.getKeyPairName() != null) {
-         options.withKeyName(keyName);        
+         options.keyPairName(templateOptions.getKeyPairName());        
-         KeyPair keyPair = keyPairCache.getIfPresent(ZoneAndName.fromZoneAndName(template.getLocation().getId(), keyName));
+         KeyPair keyPair = keyPairCache.getIfPresent(ZoneAndName.fromZoneAndName(template.getLocation().getId(), templateOptions.getKeyPairName()));
-         if (keyPair != null) {
+         if (keyPair != null && keyPair.getPrivateKey() != null) {
-            credentialsBuilder.privateKey(keyPair.getPrivateKey());
+            privateKey = Optional.of(keyPair.getPrivateKey());
+            credentialsBuilder.privateKey(privateKey.get());
          }
       }
 
       String zoneId = template.getLocation().getId();
-      Server server = novaClient.getServerClientForZone(zoneId).createServer(name, template.getImage().getProviderId(),
+      String imageId = template.getImage().getProviderId();
-               template.getHardware().getProviderId(), options);
+      String flavorId = template.getHardware().getProviderId();
+
+      logger.debug(">> creating new server zone(%s) name(%s) image(%s) flavor(%s) options(%s)", zoneId, name, imageId, flavorId, options);
+      Server server = novaClient.getServerClientForZone(zoneId).createServer(name, imageId, flavorId, options);
+      logger.trace("<< server(%s)", server.getId());
+
       ServerInZone serverInZone = new ServerInZone(server, zoneId);
+      if (!privateKey.isPresent())
+         credentialsBuilder.password(server.getAdminPass());
       return new NodeAndInitialCredentials<ServerInZone>(serverInZone, serverInZone.slashEncode(), credentialsBuilder
-            .password(server.getAdminPass()).build());
+               .build());
    }
 
    @Override

labs/openstack-nova/src/main/java/org/jclouds/openstack/nova/v1_1/options/CreateServerOptions.java

@@ -18,6 +18,8 @@
  */
 package org.jclouds.openstack.nova.v1_1.options;
 
+import static com.google.common.base.Objects.equal;
+import static com.google.common.base.Objects.toStringHelper;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Preconditions.checkState;
@@ -37,11 +39,12 @@ import org.jclouds.rest.MapBinder;
 import org.jclouds.rest.binders.BindToJsonPayload;
 import org.jclouds.util.Preconditions2;
 
+import com.google.common.base.Objects;
 import com.google.common.base.Optional;
+import com.google.common.base.Objects.ToStringHelper;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
 import com.google.gson.annotations.SerializedName;
 
@@ -76,7 +79,66 @@ public class CreateServerOptions implements MapBinder {
       public String getPath() {
          return path;
       }
+      
+      @Override
+      public boolean equals(Object object) {
+         if (this == object) {
+            return true;
+         }
+         if (object instanceof File) {
+            final File other = File.class.cast(object);
+            return equal(path, other.path);
+         } else {
+            return false;
+         }
+      }
 
+      @Override
+      public int hashCode() {
+         return Objects.hashCode(path);
+      }
+
+      @Override
+      public String toString() {
+         return toStringHelper("file").add("path", path).toString();
+      }
+
+   }
+
+   private String keyName;
+   private String adminPass;
+   private Set<String> securityGroupNames = ImmutableSet.of();
+   private Map<String, String> metadata = ImmutableMap.of();
+   private List<File> personality = Lists.newArrayList();
+
+   @Override
+   public boolean equals(Object object) {
+      if (this == object) {
+         return true;
+      }
+      if (object instanceof CreateServerOptions) {
+         final CreateServerOptions other = CreateServerOptions.class.cast(object);
+         return equal(keyName, other.keyName) && equal(securityGroupNames, other.securityGroupNames)
+                  && equal(metadata, other.metadata) && equal(personality, other.personality)
+                  && equal(adminPass, other.adminPass);
+      } else {
+         return false;
+      }
+   }
+
+   @Override
+   public int hashCode() {
+      return Objects.hashCode(keyName, securityGroupNames, metadata, personality, adminPass);
+   }
+
+   protected ToStringHelper string() {
+      return toStringHelper("").add("keyName", "keyName").add("securityGroupNames", securityGroupNames).add("metadata",
+               metadata).add("personality", personality).add("adminPassPresent", adminPass != null);
+   }
+
+   @Override
+   public String toString() {
+      return string().toString();
    }
 
    @SuppressWarnings("unused")
@@ -99,12 +161,6 @@ public class CreateServerOptions implements MapBinder {
 
    }
 
-   private Map<String, String> metadata = Maps.newHashMap();
-   private List<File> files = Lists.newArrayList();
-   private Set<String> securityGroupNames = Sets.newHashSet();
-   private String keyName;
-   private String adminPass;
-
    @Override
    public <R extends HttpRequest> R bindToRequest(R request, Map<String, String> postParams) {
       ServerRequest server = new ServerRequest(checkNotNull(postParams.get("name"), "name parameter not present"),
@@ -112,8 +168,8 @@ public class CreateServerOptions implements MapBinder {
                   postParams.get("flavorRef"), "flavorRef parameter not present"));
       if (metadata.size() > 0)
          server.metadata = metadata;
-      if (files.size() > 0)
+      if (personality.size() > 0)
-         server.personality = files;
+         server.personality = personality;
       if (keyName != null)
          server.key_name = keyName;
       if (securityGroupNames.size() > 0) {
@@ -148,13 +204,13 @@ public class CreateServerOptions implements MapBinder {
     * root group as owner and group owner, respectively and will allow user and
     * group read access only (-r--r-----).
     */
-   public CreateServerOptions withFile(String path, byte[] contents) {
+   public CreateServerOptions writeFileToPath(byte[] contents, String path) {
-      checkState(files.size() < 5, "maximum number of files allowed is 5");
+      checkState(personality.size() < 5, "maximum number of files allowed is 5");
-      files.add(new File(path, contents));
+      personality.add(new File(path, contents));
       return this;
    }
 
-   public CreateServerOptions withAdminPass(String adminPass) {
+   public CreateServerOptions adminPass(String adminPass) {
       checkNotNull(adminPass, "adminPass");
       this.adminPass = adminPass;
       return this;
@@ -167,7 +223,7 @@ public class CreateServerOptions implements MapBinder {
     * is each 255 bytes and the maximum number of key-value pairs that can be
     * supplied per server is 5.
     */
-   public CreateServerOptions withMetadata(Map<String, String> metadata) {
+   public CreateServerOptions metadata(Map<String, String> metadata) {
       checkNotNull(metadata, "metadata");
       checkArgument(metadata.size() <= 5,
             "you cannot have more then 5 metadata values.  You specified: " + metadata.size());
@@ -180,7 +236,7 @@ public class CreateServerOptions implements MapBinder {
                "maximum length of metadata value is 255 bytes.  Value specified for %s (%s) is %d bytes",
                entry.getKey(), entry.getValue(), entry.getValue().getBytes().length));
       }
-      this.metadata = metadata;
+      this.metadata = ImmutableMap.copyOf(metadata);
       return this;
    }
 
@@ -191,8 +247,14 @@ public class CreateServerOptions implements MapBinder {
     * @param keyName
     * @return
     */
-   public CreateServerOptions withKeyName(String keyName) {
+   public String getKeyPairName() {
-      checkNotNull(keyName, "keyName");
+      return keyName;
+   }
+   
+   /**
+    * @see #getKeyPairName()
+    */
+   public CreateServerOptions keyPairName(String keyName) {
       this.keyName = keyName;
       return this;
    }
@@ -232,32 +294,32 @@ public class CreateServerOptions implements MapBinder {
    public static class Builder {
 
       /**
-       * @see CreateServerOptions#withFile(String, byte[])
+       * @see CreateServerOptions#writeFileToPath
        */
-      public static CreateServerOptions withFile(String path, byte[] contents) {
+      public static CreateServerOptions writeFileToPath(byte[] contents,String path) {
          CreateServerOptions options = new CreateServerOptions();
-         return options.withFile(path, contents);
+         return options.writeFileToPath(contents, path);
       }
 
-      public static CreateServerOptions withAdminPass(String adminPass) {
+      public static CreateServerOptions adminPass(String adminPass) {
          CreateServerOptions options = new CreateServerOptions();
-         return options.withAdminPass(adminPass);
+         return options.adminPass(adminPass);
       }
 
       /**
-       * @see CreateServerOptions#withMetadata(Map<String, String>)
+       * @see CreateServerOptions#metadata(Map<String, String>)
        */
-      public static CreateServerOptions withMetadata(Map<String, String> metadata) {
+      public static CreateServerOptions metadata(Map<String, String> metadata) {
          CreateServerOptions options = new CreateServerOptions();
-         return options.withMetadata(metadata);
+         return options.metadata(metadata);
       }
 
       /**
-       * @see CreateServerOptions#withKeyName(String)
+       * @see #getKeyPairName()
        */
-      public static CreateServerOptions withKeyName(String keyName) {
+      public static CreateServerOptions keyPairName(String keyName) {
          CreateServerOptions options = new CreateServerOptions();
-         return options.withKeyName(keyName);
+         return options.keyPairName(keyName);
       }
       
       /**

labs/openstack-nova/src/test/java/org/jclouds/openstack/nova/v1_1/compute/NovaComputeServiceAdapterExpectTest.java

@@ -18,6 +18,7 @@
  */
 package org.jclouds.openstack.nova.v1_1.compute;
 
+import static org.testng.Assert.assertEquals;
 import static org.testng.Assert.assertNotNull;
 
 import java.net.URI;
@@ -27,16 +28,22 @@ import org.jclouds.compute.ComputeServiceContext;
 import org.jclouds.compute.ComputeServiceAdapter.NodeAndInitialCredentials;
 import org.jclouds.compute.domain.Template;
 import org.jclouds.compute.domain.TemplateBuilder;
+import org.jclouds.domain.LoginCredentials;
 import org.jclouds.http.HttpRequest;
 import org.jclouds.http.HttpResponse;
 import org.jclouds.openstack.nova.v1_1.compute.options.NovaTemplateOptions;
+import org.jclouds.openstack.nova.v1_1.domain.KeyPair;
 import org.jclouds.openstack.nova.v1_1.domain.zonescoped.ServerInZone;
+import org.jclouds.openstack.nova.v1_1.domain.zonescoped.ZoneAndName;
 import org.jclouds.openstack.nova.v1_1.internal.BaseNovaComputeServiceContextExpectTest;
 import org.testng.annotations.Test;
 
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableMultimap;
 import com.google.inject.Injector;
+import com.google.inject.Key;
+import com.google.inject.TypeLiteral;
 
 /**
  * Tests the compute service abstraction of the nova client.
@@ -81,8 +88,58 @@ public class NovaComputeServiceAdapterExpectTest extends BaseNovaComputeServiceC
       NodeAndInitialCredentials<ServerInZone> server = adapter.createNodeWithGroupEncodedIntoName("test", "test-e92",
                template);
       assertNotNull(server);
+      assertEquals(server.getCredentials(), LoginCredentials.builder().password("ZWuHcmTMQ7eXoHeM").build());
    }
 
+   /**
+    * We need to choose the correct credential for attempts to start the server. cloud-init or the
+    * like will set the ssh key as the login credential, and not necessarily will password
+    * authentication even be available.
+    */
+   public void testWhenKeyPairPresentWeUsePrivateKeyAsCredentialNotPassword() throws Exception {
+      
+      HttpRequest createServer = HttpRequest
+         .builder()
+         .method("POST")
+         .endpoint(URI.create("https://compute.north.host/v1.1/3456/servers"))
+         .headers(
+               ImmutableMultimap.<String, String> builder().put("Accept", "application/json")
+                     .put("X-Auth-Token", authToken).build())
+         .payload(payloadFromStringWithContentType(
+                  "{\"server\":{\"name\":\"test-e92\",\"imageRef\":\"1241\",\"flavorRef\":\"100\",\"key_name\":\"foo\"}}","application/json"))
+         .build();
+
+  
+      HttpResponse createServerResponse = HttpResponse.builder().statusCode(202).message("HTTP/1.1 202 Accepted")
+         .payload(payloadFromResourceWithContentType("/new_server.json","application/json; charset=UTF-8")).build();
+
+         
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+               .put(keystoneAuthWithAccessKeyAndSecretKey, responseWithKeystoneAccess)
+               .put(extensionsOfNovaRequest, extensionsOfNovaResponse)
+               .put(listImagesDetail, listImagesDetailResponse)
+               .put(listFlavorsDetail, listFlavorsDetailResponse)
+               .put(createServer, createServerResponse).build();
+
+      Injector forSecurityGroups = requestsSendResponses(requestResponseMap);
+
+      Template template = forSecurityGroups.getInstance(TemplateBuilder.class).build();
+      template.getOptions().as(NovaTemplateOptions.class).keyPairName("foo");
+      
+      NovaComputeServiceAdapter adapter = forSecurityGroups.getInstance(NovaComputeServiceAdapter.class);
+      
+      // we expect to have already an entry in the cache for the key
+      LoadingCache<ZoneAndName, KeyPair> keyPairCache = forSecurityGroups.getInstance(Key
+               .get(new TypeLiteral<LoadingCache<ZoneAndName, KeyPair>>() {
+               }));
+      keyPairCache.put(ZoneAndName.fromZoneAndName("az-1.region-a.geo-1", "foo"), KeyPair.builder().name("foo")
+               .privateKey("privateKey").build());
+      
+      NodeAndInitialCredentials<ServerInZone> server = adapter.createNodeWithGroupEncodedIntoName("test", "test-e92",
+               template);
+      assertNotNull(server);
+      assertEquals(server.getCredentials(), LoginCredentials.builder().privateKey("privateKey").build());
+   }
    @Override
    public Injector apply(ComputeServiceContext input) {
       return input.utils().injector();