Issue 1121: Don't generate SecurityGroup when user specifies NovaTemplateOptions.securityGroupNames

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java

@@ -105,7 +105,8 @@ public class NovaComputeServiceAdapter implements
 
       CreateServerOptions options = new CreateServerOptions();
       options.metadata(metadataAndTagsAsCommaDelimitedValue(template.getOptions()));
-      options.securityGroupNames(templateOptions.getSecurityGroupNames());
+      if (templateOptions.getSecurityGroupNames().isPresent())
+         options.securityGroupNames(templateOptions.getSecurityGroupNames().get());
       options.userData(templateOptions.getUserData());
 
       Optional<String> privateKey = Optional.absent();

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java

@@ -32,6 +32,7 @@ import org.jclouds.scriptbuilder.domain.Statement;
 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.ImmutableSet;
 
@@ -65,7 +66,8 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable {
       if (to instanceof NovaTemplateOptions) {
          NovaTemplateOptions eTo = NovaTemplateOptions.class.cast(to);
          eTo.autoAssignFloatingIp(shouldAutoAssignFloatingIp());
-         eTo.securityGroupNames(getSecurityGroupNames());
+         if (getSecurityGroupNames().isPresent())
+            eTo.securityGroupNames(getSecurityGroupNames().get());
          eTo.generateKeyPair(shouldGenerateKeyPair());
          eTo.keyPairName(getKeyPairName());
          if (getUserData() != null) {
@@ -75,7 +77,7 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable {
    }
 
    protected boolean autoAssignFloatingIp = false;
-   protected Set<String> securityGroupNames = ImmutableSet.of();
+   protected Optional<Set<String>> securityGroupNames = Optional.absent();
    protected boolean generateKeyPair = false;
    protected String keyPairName;
    protected byte[] userData;
@@ -104,8 +106,8 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable {
       ToStringHelper toString = super.string();
       if (!autoAssignFloatingIp)
          toString.add("autoAssignFloatingIp", autoAssignFloatingIp);
-      if (securityGroupNames.size() != 0)
+      if (securityGroupNames.isPresent())
-         toString.add("securityGroupNames", securityGroupNames);
+         toString.add("securityGroupNames", securityGroupNames.get());
       if (generateKeyPair)
          toString.add("generateKeyPair", generateKeyPair);
       toString.add("keyPairName", keyPairName);
@@ -153,7 +155,7 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable {
    public NovaTemplateOptions securityGroupNames(Iterable<String> securityGroupNames) {
       for (String groupName : checkNotNull(securityGroupNames, "securityGroupNames"))
          Preconditions2.checkNotEmpty(groupName, "all security groups must be non-empty");
-      this.securityGroupNames = ImmutableSet.copyOf(securityGroupNames);
+      this.securityGroupNames = Optional.<Set<String>> of(ImmutableSet.copyOf(securityGroupNames));
       return this;
    }
 
@@ -190,9 +192,12 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable {
    }
    
    /**
+    * if unset, generate a default group prefixed with {@link jclouds#} according
+    * to {@link #getInboundPorts()}
+    * 
     * @see org.jclouds.openstack.nova.v2_0.options.CreateServerOptions#getSecurityGroupNames
     */
-   public Set<String> getSecurityGroupNames() {
+   public Optional<Set<String>> getSecurityGroupNames() {
       return securityGroupNames;
    }
 

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java

@@ -128,18 +128,21 @@ public class ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddT
 
       boolean securityGroupExtensionPresent = novaApi.getSecurityGroupExtensionForZone(zone).isPresent();
       List<Integer> inboundPorts = Ints.asList(templateOptions.getInboundPorts());
-      if (templateOptions.getSecurityGroupNames().size() > 0) {
+      if (templateOptions.getSecurityGroupNames().isPresent()
-         checkArgument(novaApi.getSecurityGroupExtensionForZone(zone).isPresent(),
+            && templateOptions.getSecurityGroupNames().get().size() > 0) {
+         checkArgument(securityGroupExtensionPresent,
                   "Security groups are required by options, but the extension is not available! options: %s",
                   templateOptions);
-      } else if (securityGroupExtensionPresent && inboundPorts.size() > 0) {
+      } else if (securityGroupExtensionPresent) {
-         String securityGroupName = namingConvention.create().sharedNameForGroup(group);
+         if (!templateOptions.getSecurityGroupNames().isPresent() && inboundPorts.size() > 0) {
-         try {
+            String securityGroupName = namingConvention.create().sharedNameForGroup(group);
-            securityGroupCache.get(new ZoneSecurityGroupNameAndPorts(zone, securityGroupName, inboundPorts));
+            try {
-         } catch (ExecutionException e) {
+               securityGroupCache.get(new ZoneSecurityGroupNameAndPorts(zone, securityGroupName, inboundPorts));
-            throw Throwables.propagate(e.getCause());
+            } catch (ExecutionException e) {
+               throw Throwables.propagate(e.getCause());
+            }
+            templateOptions.securityGroupNames(securityGroupName);
          }
-         templateOptions.securityGroupNames(securityGroupName);
       }
 
       return super.execute(group, count, mutableTemplate, goodNodes, badNodes, customizationResponses);

apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java

@@ -291,14 +291,13 @@ public class CreateServerOptions implements MapBinder {
    }
    
    /**
+    * 
+    * Security groups the user specified to run servers with.
     * 
     * <h3>Note</h3>
     * 
     * This requires that {@link NovaApi#getSecurityGroupExtensionForZone(String)} to return
     * {@link Optional#isPresent present}
-    * 
-    * @return security groups the user specified to run servers with; zero length will create an
-    *         implicit group starting with {@code jclouds#}
     */
    public Set<String> getSecurityGroupNames() {
       return securityGroupNames;

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceExpectTest.java

@@ -328,4 +328,54 @@ public class NovaComputeServiceExpectTest extends BaseNovaComputeServiceExpectTe
       assertEquals(node.getCredentials().getPrivateKey(), null);
    }
 
+
+   @Test
+   public void testCreateNodeWhileUserSpecifiesKeyPairAndUserSpecifiedGroups() throws Exception {
+      Builder<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+            .putAll(defaultTemplateTryStack);
+      requestResponseMap.put(list, notFound);
+
+      requestResponseMap.put(serverDetail, serverDetailResponse);
+
+      HttpRequest createServerWithSuppliedKeyPairAndGroup = HttpRequest
+            .builder()
+            .method("POST")
+            .endpoint("https://nova-api.trystack.org:9774/v1.1/3456/servers")
+            .addHeader("Accept", "application/json")
+            .addHeader("X-Auth-Token", authToken)
+            .payload(
+                  payloadFromStringWithContentType(
+                        "{\"server\":{\"name\":\"test-0\",\"imageRef\":\"14\",\"flavorRef\":\"1\",\"key_name\":\"fooPair\",\"security_groups\":[{\"name\":\"mygroup\"}]}}",
+                        "application/json")).build();
+
+      HttpResponse createdServer = HttpResponse.builder().statusCode(202).message("HTTP/1.1 202 Accepted")
+            .payload(payloadFromResourceWithContentType("/new_server.json", "application/json; charset=UTF-8")).build();
+
+      requestResponseMap.put(createServerWithSuppliedKeyPairAndGroup, createdServer);
+
+      ComputeService apiThatCreatesNode = requestsSendResponses(requestResponseMap.build(), new AbstractModule() {
+
+         @Override
+         protected void configure() {
+            // predicatable node names
+            final AtomicInteger suffix = new AtomicInteger();
+            bind(new TypeLiteral<Supplier<String>>() {
+            }).toInstance(new Supplier<String>() {
+
+               @Override
+               public String get() {
+                  return suffix.getAndIncrement() + "";
+               }
+
+            });
+         }
+
+      });
+
+      NodeMetadata node = Iterables.getOnlyElement(apiThatCreatesNode.createNodesInGroup("test", 1,
+            keyPairName("fooPair").securityGroupNames("mygroup").blockUntilRunning(false)));
+      // we don't have access to this private key
+      assertEquals(node.getCredentials().getPrivateKey(), null);
+   }
+
 }

apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptionsTest.java

@@ -32,6 +32,7 @@ import java.io.IOException;
 import org.jclouds.compute.options.TemplateOptions;
 import org.testng.annotations.Test;
 
+import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableSet;
 
 /**
@@ -57,14 +58,14 @@ public class NovaTemplateOptionsTest {
    public void testsecurityGroupNamesIterable() {
       NovaTemplateOptions options = new NovaTemplateOptions();
       options.securityGroupNames(ImmutableSet.of("group1", "group2"));
-      assertEquals(options.getSecurityGroupNames(), ImmutableSet.of("group1", "group2"));
+      assertEquals(options.getSecurityGroupNames(), Optional.of(ImmutableSet.of("group1", "group2")));
 
    }
 
    @Test
    public void testsecurityGroupNamesIterableStatic() {
       NovaTemplateOptions options = securityGroupNames(ImmutableSet.of("group1", "group2"));
-      assertEquals(options.getSecurityGroupNames(), ImmutableSet.of("group1", "group2"));
+      assertEquals(options.getSecurityGroupNames(), Optional.of(ImmutableSet.of("group1", "group2")));
    }
 
    @Test(expectedExceptions = IllegalArgumentException.class)
@@ -77,20 +78,20 @@ public class NovaTemplateOptionsTest {
    public void testsecurityGroupNamesVarArgs() {
       NovaTemplateOptions options = new NovaTemplateOptions();
       options.securityGroupNames("group1", "group2");
-      assertEquals(options.getSecurityGroupNames(), ImmutableSet.of("group1", "group2"));
+      assertEquals(options.getSecurityGroupNames(), Optional.of(ImmutableSet.of("group1", "group2")));
 
    }
 
    @Test
    public void testDefaultGroupsVarArgsEmpty() {
       NovaTemplateOptions options = new NovaTemplateOptions();
-      assertEquals(options.getSecurityGroupNames(), ImmutableSet.of());
+      assertEquals(options.getSecurityGroupNames(), Optional.absent());
    }
 
    @Test
    public void testsecurityGroupNamesVarArgsStatic() {
       NovaTemplateOptions options = securityGroupNames("group1", "group2");
-      assertEquals(options.getSecurityGroupNames(), ImmutableSet.of("group1", "group2"));
+      assertEquals(options.getSecurityGroupNames(), Optional.of(ImmutableSet.of("group1", "group2")));
    }
 
    @Test