From caa8fdee2066b036973fc390d7eaf35f31ddedef Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 12 Nov 2012 22:33:36 -0800 Subject: [PATCH] Issue 1121: Don't generate SecurityGroup when user specifies NovaTemplateOptions.securityGroupNames --- .../compute/NovaComputeServiceAdapter.java | 3 +- .../compute/options/NovaTemplateOptions.java | 16 +++--- ...sWithGroupEncodedIntoNameThenAddToSet.java | 21 ++++---- .../v2_0/options/CreateServerOptions.java | 5 +- .../compute/NovaComputeServiceExpectTest.java | 50 +++++++++++++++++++ .../options/NovaTemplateOptionsTest.java | 11 ++-- 6 files changed, 82 insertions(+), 24 deletions(-) diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java index f934682efe..145871d9a4 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceAdapter.java @@ -106,7 +106,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 privateKey = Optional.absent(); diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java index 32aa332546..a7699f4145 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptions.java @@ -67,7 +67,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) { @@ -77,7 +78,7 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable { } protected boolean autoAssignFloatingIp = false; - protected Set securityGroupNames = ImmutableSet.of(); + protected Optional> securityGroupNames = Optional.absent(); protected boolean generateKeyPair = false; protected String keyPairName; protected byte[] userData; @@ -106,8 +107,8 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable { ToStringHelper toString = super.string(); if (!autoAssignFloatingIp) toString.add("autoAssignFloatingIp", autoAssignFloatingIp); - if (securityGroupNames.size() != 0) - toString.add("securityGroupNames", securityGroupNames); + if (securityGroupNames.isPresent()) + toString.add("securityGroupNames", securityGroupNames.get()); if (generateKeyPair) toString.add("generateKeyPair", generateKeyPair); toString.add("keyPairName", keyPairName); @@ -155,7 +156,7 @@ public class NovaTemplateOptions extends TemplateOptions implements Cloneable { public NovaTemplateOptions securityGroupNames(Iterable 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.> of(ImmutableSet.copyOf(securityGroupNames)); return this; } @@ -192,9 +193,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 getSecurityGroupNames() { + public Optional> getSecurityGroupNames() { return securityGroupNames; } diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java index dbb184117e..85fd629f26 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/strategy/ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet.java +++ b/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 inboundPorts = Ints.asList(templateOptions.getInboundPorts()); - if (templateOptions.getSecurityGroupNames().size() > 0) { - checkArgument(novaApi.getSecurityGroupExtensionForZone(zone).isPresent(), + if (templateOptions.getSecurityGroupNames().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) { - String securityGroupName = namingConvention.create().sharedNameForGroup(group); - try { - securityGroupCache.get(new ZoneSecurityGroupNameAndPorts(zone, securityGroupName, inboundPorts)); - } catch (ExecutionException e) { - throw Throwables.propagate(e.getCause()); + } else if (securityGroupExtensionPresent) { + if (!templateOptions.getSecurityGroupNames().isPresent() && inboundPorts.size() > 0) { + String securityGroupName = namingConvention.create().sharedNameForGroup(group); + try { + securityGroupCache.get(new ZoneSecurityGroupNameAndPorts(zone, securityGroupName, inboundPorts)); + } catch (ExecutionException e) { + throw Throwables.propagate(e.getCause()); + } + templateOptions.securityGroupNames(securityGroupName); } - templateOptions.securityGroupNames(securityGroupName); } return super.execute(group, count, mutableTemplate, goodNodes, badNodes, customizationResponses); diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java index db2bbed3ee..180a92399a 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/options/CreateServerOptions.java @@ -293,14 +293,13 @@ public class CreateServerOptions implements MapBinder { } /** + * + * Security groups the user specified to run servers with. * *

Note

* * 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 getSecurityGroupNames() { return securityGroupNames; diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceExpectTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceExpectTest.java index 94ae928f06..04cdc36499 100644 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/NovaComputeServiceExpectTest.java +++ b/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 requestResponseMap = ImmutableMap. 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>() { + }).toInstance(new Supplier() { + + @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); + } + } diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptionsTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptionsTest.java index 2eb02a3b33..20f494cf50 100644 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/options/NovaTemplateOptionsTest.java +++ b/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