From 717b75a34e261689f900c7f623088a866932a33e Mon Sep 17 00:00:00 2001 From: Geoff Macartney Date: Wed, 1 Feb 2017 10:04:32 +0000 Subject: [PATCH] Fix O(n^2) response time for "list-security-groups" on openstack-nova. For https://issues.apache.org/jira/browse/JCLOUDS-1235. This change takes the approach of storing the information about the overall list of groups within the `SecurityGroupInRegion` when it is created, so that any subsequent conversion operation has access to all the groups in the same region as the one to be converted. It also collapses the functionality of `NovaSecurityGroupToSecurityGroup`, `SecurityGroupRuleToIpPermission` and `FindSecurityGroupWithNameAndReturnTrue` all into `NovaSecurityGroupInRegionToSecurityGroup`, and deletes the now unused-classes SecurityGroupRuleToIpPermission, NovaSecurityGroupToSecurityGroup and associated tests. --- .../NovaComputeServiceContextModule.java | 10 -- .../NovaSecurityGroupExtension.java | 18 ++- .../CreateSecurityGroupIfNeeded.java | 16 +- ...aSecurityGroupInRegionToSecurityGroup.java | 103 +++++++++--- .../NovaSecurityGroupToSecurityGroup.java | 66 -------- .../SecurityGroupRuleToIpPermission.java | 96 ----------- .../regionscoped/SecurityGroupInRegion.java | 34 +++- ...indSecurityGroupWithNameAndReturnTrue.java | 6 +- .../NovaSecurityGroupExtensionExpectTest.java | 87 ++++++++-- .../NovaSecurityGroupExtensionLiveTest.java | 86 ++++++++++ ...urityGroupInRegionToSecurityGroupTest.java | 88 ++++++++-- .../NovaSecurityGroupToSecurityGroupTest.java | 152 ------------------ .../SecurityGroupRuleToIpPermissionTest.java | 79 --------- .../CreateSecurityGroupIfNeededTest.java | 30 ++-- ...yGroupWithNameAndReturnTrueExpectTest.java | 9 +- .../rest/internal/BaseRestApiExpectTest.java | 3 +- 16 files changed, 399 insertions(+), 484 deletions(-) delete mode 100644 apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroup.java delete mode 100644 apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermission.java delete mode 100644 apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroupTest.java delete mode 100644 apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermissionTest.java diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java index 477ff0aaf1..cead92bdb1 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/config/NovaComputeServiceContextModule.java @@ -46,7 +46,6 @@ import org.jclouds.compute.strategy.impl.CreateNodesWithGroupEncodedIntoNameThen import org.jclouds.domain.Location; import org.jclouds.domain.LoginCredentials; import org.jclouds.functions.IdentityFunction; -import org.jclouds.net.domain.IpPermission; import org.jclouds.openstack.nova.v2_0.compute.NovaComputeService; import org.jclouds.openstack.nova.v2_0.compute.NovaComputeServiceAdapter; import org.jclouds.openstack.nova.v2_0.compute.extensions.NovaImageExtension; @@ -57,9 +56,7 @@ import org.jclouds.openstack.nova.v2_0.compute.functions.FlavorInRegionToHardwar import org.jclouds.openstack.nova.v2_0.compute.functions.ImageInRegionToImage; import org.jclouds.openstack.nova.v2_0.compute.functions.ImageToOperatingSystem; import org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupInRegionToSecurityGroup; -import org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroup; import org.jclouds.openstack.nova.v2_0.compute.functions.OrphanedGroupsByRegionId; -import org.jclouds.openstack.nova.v2_0.compute.functions.SecurityGroupRuleToIpPermission; import org.jclouds.openstack.nova.v2_0.compute.functions.ServerInRegionToNodeMetadata; import org.jclouds.openstack.nova.v2_0.compute.loaders.CreateUniqueKeyPair; import org.jclouds.openstack.nova.v2_0.compute.loaders.FindSecurityGroupOrCreate; @@ -68,7 +65,6 @@ import org.jclouds.openstack.nova.v2_0.compute.options.NovaTemplateOptions; import org.jclouds.openstack.nova.v2_0.compute.strategy.ApplyNovaTemplateOptionsCreateNodesWithGroupEncodedIntoNameThenAddToSet; import org.jclouds.openstack.nova.v2_0.domain.FloatingIP; import org.jclouds.openstack.nova.v2_0.domain.KeyPair; -import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; import org.jclouds.openstack.nova.v2_0.domain.Server; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.FlavorInRegion; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.ImageInRegion; @@ -114,12 +110,6 @@ public class NovaComputeServiceContextModule extends bind(new TypeLiteral>() { }).to(ServerInRegionToNodeMetadata.class); - bind(new TypeLiteral>() { - }).to(SecurityGroupRuleToIpPermission.class); - - bind(new TypeLiteral>() { - }).to(NovaSecurityGroupToSecurityGroup.class); - bind(new TypeLiteral>() { }).to(NovaSecurityGroupInRegionToSecurityGroup.class); diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java index 1a5c4fe689..88ab63cb21 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/extensions/NovaSecurityGroupExtension.java @@ -58,6 +58,7 @@ 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.FluentIterable; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.google.common.util.concurrent.ListeningExecutorService; @@ -135,8 +136,9 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension { } Set groupNames = instance.getSecurityGroupNames(); - Set rawGroups = - sgApi.get().list().filter(nameIn(groupNames)).transform(groupToGroupInRegion(region)).toSet(); + final FluentIterable allGroups = sgApi.get().list(); + Set rawGroups = + allGroups.filter(nameIn(groupNames)).transform(groupToGroupInRegion(allGroups, region)).toSet(); return ImmutableSet.copyOf(transform(filter(rawGroups, notNull()), groupConverter)); } @@ -153,7 +155,8 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension { return null; } - SecurityGroupInRegion rawGroup = new SecurityGroupInRegion(sgApi.get().get(groupId), region); + final FluentIterable allGroups = sgApi.get().list(); + SecurityGroupInRegion rawGroup = new SecurityGroupInRegion(sgApi.get().get(groupId), region, allGroups); return groupConverter.apply(rawGroup); } @@ -359,17 +362,20 @@ public class NovaSecurityGroupExtension implements SecurityGroupExtension { } - return sgApi.get().list().transform(groupToGroupInRegion(from)).toSet(); + final FluentIterable allGroups = sgApi.get().list(); + return allGroups.transform(groupToGroupInRegion(allGroups, from)).toSet(); } }; } - protected Function groupToGroupInRegion(final String region) { + protected Function groupToGroupInRegion( + final Iterable allGroups, final String region) { + return new Function() { @Override public SecurityGroupInRegion apply(org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group) { - return new SecurityGroupInRegion(group, region); + return new SecurityGroupInRegion(group, region, allGroups); } }; } diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java index 4e9b11cb1d..deea2e917f 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/CreateSecurityGroupIfNeeded.java @@ -38,6 +38,7 @@ import org.jclouds.openstack.nova.v2_0.extensions.SecurityGroupApi; import com.google.common.base.Function; import com.google.common.base.Optional; +import com.google.common.collect.FluentIterable; @Singleton public class CreateSecurityGroupIfNeeded implements Function { @@ -58,31 +59,30 @@ public class CreateSecurityGroupIfNeeded implements Function api = novaApi.getSecurityGroupApi(regionId); checkArgument(api.isPresent(), "Security groups are required, but the extension is not available in region %s!", regionId); + final FluentIterable allGroups = api.get().list(); logger.debug(">> creating securityGroup %s", regionSecurityGroupNameAndPorts); try { - SecurityGroup securityGroup = api.get().createWithDescription( - regionSecurityGroupNameAndPorts.getName(), regionSecurityGroupNameAndPorts.getName()); + regionSecurityGroupNameAndPorts.getName(), regionSecurityGroupNameAndPorts.getName()); logger.debug("<< created securityGroup(%s)", securityGroup); for (int port : regionSecurityGroupNameAndPorts.getPorts()) { authorizeGroupToItselfAndAllIPsToTCPPort(api.get(), securityGroup, port); } - return new SecurityGroupInRegion(api.get().get(securityGroup.getId()), regionId); + return new SecurityGroupInRegion(api.get().get(securityGroup.getId()), regionId, allGroups); } catch (IllegalStateException e) { logger.trace("<< trying to find securityGroup(%s): %s", regionSecurityGroupNameAndPorts, e.getMessage()); - SecurityGroup group = find(api.get().list(), nameEquals(regionSecurityGroupNameAndPorts - .getName())); + SecurityGroup group = find(allGroups, nameEquals(regionSecurityGroupNameAndPorts.getName())); logger.debug("<< reused securityGroup(%s)", group.getId()); - return new SecurityGroupInRegion(group, regionId); + return new SecurityGroupInRegion(group, regionId, allGroups); } } private void authorizeGroupToItselfAndAllIPsToTCPPort(SecurityGroupApi securityGroupApi, - SecurityGroup securityGroup, int port) { + SecurityGroup securityGroup, int port) { logger.debug(">> authorizing securityGroup(%s) permission to 0.0.0.0/0 on port %d", securityGroup, port); securityGroupApi.createRuleAllowingCidrBlock(securityGroup.getId(), Ingress.builder().ipProtocol( - IpProtocol.TCP).fromPort(port).toPort(port).build(), "0.0.0.0/0"); + IpProtocol.TCP).fromPort(port).toPort(port).build(), "0.0.0.0/0"); logger.debug("<< authorized securityGroup(%s) permission to 0.0.0.0/0 on port %d", securityGroup, port); } diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroup.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroup.java index 33b641e48f..184e9ab4b1 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroup.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroup.java @@ -18,7 +18,10 @@ package org.jclouds.openstack.nova.v2_0.compute.functions; import static com.google.common.base.Preconditions.checkNotNull; import static com.google.common.base.Preconditions.checkState; +import static com.google.common.collect.Iterables.filter; +import static com.google.common.collect.Iterables.transform; +import java.util.Collection; import java.util.Map; import javax.annotation.Resource; @@ -30,43 +33,103 @@ import org.jclouds.compute.domain.SecurityGroupBuilder; import org.jclouds.compute.reference.ComputeServiceConstants; import org.jclouds.domain.Location; import org.jclouds.logging.Logger; +import org.jclouds.net.domain.IpPermission; +import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; +import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; import com.google.common.base.Function; +import com.google.common.base.Predicates; import com.google.common.base.Supplier; +import com.google.common.collect.Iterables; import com.google.inject.Inject; + /** * A function for transforming a Nova-specific SecurityGroup into a generic * SecurityGroup object. */ @Singleton public class NovaSecurityGroupInRegionToSecurityGroup implements Function { - @Resource - @Named(ComputeServiceConstants.COMPUTE_LOGGER) - protected Logger logger = Logger.NULL; + @Resource + @Named(ComputeServiceConstants.COMPUTE_LOGGER) + protected Logger logger = Logger.NULL; - protected final Function baseConverter; - protected final Supplier> locationIndex; + protected final Supplier> locationIndex; - @Inject - public NovaSecurityGroupInRegionToSecurityGroup(Function baseConverter, - Supplier> locationIndex) { - this.baseConverter = checkNotNull(baseConverter, "baseConverter"); - this.locationIndex = checkNotNull(locationIndex, "locationIndex"); - } + @Inject + public NovaSecurityGroupInRegionToSecurityGroup(Supplier> locationIndex) { + this.locationIndex = checkNotNull(locationIndex, "locationIndex"); + } - @Override - public SecurityGroup apply(SecurityGroupInRegion group) { - SecurityGroupBuilder builder = SecurityGroupBuilder.fromSecurityGroup(baseConverter.apply(group.getSecurityGroup())); + @Override + public SecurityGroup apply(final SecurityGroupInRegion groupInRegion) { + SecurityGroupBuilder builder = new SecurityGroupBuilder(); - Location region = locationIndex.get().get(group.getRegion()); - checkState(region != null, "location %s not in locationIndex: %s", group.getRegion(), locationIndex.get()); + final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group = groupInRegion.getSecurityGroup(); + builder.id(group.getId()); + builder.providerId(group.getId()); + builder.ownerId(group.getTenantId()); + builder.name(group.getName()); + if (group.getRules() != null) { + builder.ipPermissions(filter(transform(group.getRules(), new Function() { + @Override + public IpPermission apply(SecurityGroupRule input) { + return securityGroupRuleToIpPermission(groupInRegion, input); + } + }), Predicates.notNull())); + } - builder.location(region); + final String regionId = groupInRegion.getRegion(); + Location region = locationIndex.get().get(regionId); + checkState(region != null, "location %s not in locationIndex: %s", regionId, locationIndex.get()); - builder.id(group.getRegion() + "/" + group.getSecurityGroup().getId()); + builder.location(region); - return builder.build(); - } + builder.id(regionId + "/" + group.getId()); + + return builder.build(); + } + + private IpPermission securityGroupRuleToIpPermission(SecurityGroupInRegion groupInRegion, SecurityGroupRule rule) { + IpPermission.Builder builder = IpPermission.builder(); + builder.ipProtocol(rule.getIpProtocol()); + builder.fromPort(rule.getFromPort()); + builder.toPort(rule.getToPort()); + final TenantIdAndName ruleGroup = rule.getGroup(); + if (ruleGroup != null) { + final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup owningGroup = + groupInRegion.getSecurityGroup(); + final Collection referredGroup = + groupInRegion.getGroupsByName().get(ruleGroup); + if (null == referredGroup) { + logger.warn("Unknown group {} used in security rule, refusing to add it to {} ({})", + ruleGroup, owningGroup.getName(), owningGroup.getId()); + return null; + } + /* Checking referredGroup.size(), see comments on SecurityGroupInRegion.getGroupsByName(). If there are + duplicate groups with the same tenant-id-and-name as that of ruleGroup then it is not possible + with the Nova /v2/12345/os-security-groups API to know which group is intended, as the only + information it returns about referred groups is the tenant id and name: + "group": { + "tenant_id": "a0ade3ca76784719845363979dc1014e", + "name": "jclouds-qa-scheduler-docker-entity" + }, + Rather than pick one group at random and risk using the wrong group, here we fall back to the + least-worst option(?) and refuse to add any rule. + + See https://issues.apache.org/jira/browse/JCLOUDS-1234. + */ + if (referredGroup.size() != 1) { + logger.warn("Ambiguous group %s used in security rule, refusing to add it to %s (%s)", + ruleGroup, owningGroup.getName(), owningGroup.getId()); + return null; + } + builder.groupId(groupInRegion.getRegion() + "/" + Iterables.getOnlyElement(referredGroup).getId()); + } + if (rule.getIpRange() != null) { + builder.cidrBlock(rule.getIpRange()); + } + return builder.build(); + } } diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroup.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroup.java deleted file mode 100644 index bdb7ba6a63..0000000000 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroup.java +++ /dev/null @@ -1,66 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.jclouds.openstack.nova.v2_0.compute.functions; - -import static com.google.common.collect.Iterables.transform; - -import javax.annotation.Resource; -import javax.inject.Named; -import javax.inject.Singleton; - -import org.jclouds.compute.domain.SecurityGroup; -import org.jclouds.compute.domain.SecurityGroupBuilder; -import org.jclouds.compute.reference.ComputeServiceConstants; -import org.jclouds.logging.Logger; -import org.jclouds.net.domain.IpPermission; -import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; - -import com.google.common.base.Function; -import com.google.inject.Inject; - -/** - * A function for transforming a Nova-specific SecurityGroup into a generic - * SecurityGroup object. - */ -@Singleton -public class NovaSecurityGroupToSecurityGroup implements Function { - @Resource - @Named(ComputeServiceConstants.COMPUTE_LOGGER) - protected Logger logger = Logger.NULL; - - protected Function ruleToPermission; - - @Inject - public NovaSecurityGroupToSecurityGroup(Function ruleToPermission) { - this.ruleToPermission = ruleToPermission; - } - - @Override - public SecurityGroup apply(org.jclouds.openstack.nova.v2_0.domain.SecurityGroup group) { - SecurityGroupBuilder builder = new SecurityGroupBuilder(); - - builder.id(group.getId()); - builder.providerId(group.getId()); - builder.ownerId(group.getTenantId()); - builder.name(group.getName()); - if (group.getRules() != null) { - builder.ipPermissions(transform(group.getRules(), ruleToPermission)); - } - - return builder.build(); - } -} diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermission.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermission.java deleted file mode 100644 index 6dddcc8ac7..0000000000 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermission.java +++ /dev/null @@ -1,96 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.jclouds.openstack.nova.v2_0.compute.functions; - -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.collect.Iterables.filter; -import static com.google.common.collect.Iterables.getFirst; - -import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; - -import javax.annotation.Resource; -import javax.inject.Inject; -import javax.inject.Named; - -import org.jclouds.compute.reference.ComputeServiceConstants; -import org.jclouds.domain.Location; -import org.jclouds.logging.Logger; -import org.jclouds.net.domain.IpPermission; -import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; -import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName; -import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; - -import com.google.common.base.Function; -import com.google.common.base.Predicate; -import com.google.common.base.Supplier; -import com.google.common.cache.LoadingCache; -import com.google.common.util.concurrent.Atomics; - -/** - * A function for transforming a nova-specific SecurityGroupRule into a generic - * IpPermission object. - */ -public class SecurityGroupRuleToIpPermission implements Function { - @Resource - @Named(ComputeServiceConstants.COMPUTE_LOGGER) - protected Logger logger = Logger.NULL; - protected final Predicate> returnSecurityGroupExistsInRegion; - protected final Supplier> locationIndex; - LoadingCache groupMap; - - @Inject - public SecurityGroupRuleToIpPermission(@Named("SECURITYGROUP_PRESENT") Predicate> returnSecurityGroupExistsInRegion, - Supplier> locationIndex, - LoadingCache groupMap) { - this.returnSecurityGroupExistsInRegion = checkNotNull(returnSecurityGroupExistsInRegion, - "returnSecurityGroupExistsInRegion"); - this.locationIndex = checkNotNull(locationIndex, "locationIndex"); - this.groupMap = checkNotNull(groupMap, "groupMap"); - } - - @Override - public IpPermission apply(SecurityGroupRule rule) { - IpPermission.Builder builder = IpPermission.builder(); - builder.ipProtocol(rule.getIpProtocol()); - builder.fromPort(rule.getFromPort()); - builder.toPort(rule.getToPort()); - if (rule.getGroup() != null) { - String region = getFirst(filter(locationIndex.get().keySet(), isSecurityGroupInRegion(rule.getGroup().getName())), - null); - if (region != null) { - SecurityGroupInRegion group = groupMap.getUnchecked(RegionAndName.fromRegionAndName(region, rule.getGroup().getName())); - builder.groupId(region + "/" + group.getSecurityGroup().getId()); - } - } - if (rule.getIpRange() != null) - builder.cidrBlock(rule.getIpRange()); - - return builder.build(); - } - - protected Predicate isSecurityGroupInRegion(final String groupName) { - return new Predicate() { - - @Override - public boolean apply(String region) { - AtomicReference securityGroupInRegionRef = Atomics.newReference(RegionAndName.fromRegionAndName(region, groupName)); - return returnSecurityGroupExistsInRegion.apply(securityGroupInRegionRef); - } - }; - } -} diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/SecurityGroupInRegion.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/SecurityGroupInRegion.java index 88168d4ddd..41466c44d0 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/SecurityGroupInRegion.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/domain/regionscoped/SecurityGroupInRegion.java @@ -18,28 +18,58 @@ package org.jclouds.openstack.nova.v2_0.domain.regionscoped; import static com.google.common.base.Preconditions.checkNotNull; +import java.util.Collection; +import java.util.Map; + import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup; +import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName; import com.google.common.base.Objects.ToStringHelper; +import com.google.common.collect.HashMultimap; +import com.google.common.collect.Multimap; public class SecurityGroupInRegion extends RegionAndName { protected final SecurityGroup securityGroup; - public SecurityGroupInRegion(SecurityGroup securityGroup, String regionId) { + protected final Multimap groupsByName; + + public SecurityGroupInRegion(SecurityGroup securityGroup, String regionId, Iterable allGroupsInRegion) { super(regionId, checkNotNull(securityGroup, "securityGroup").getName()); this.securityGroup = securityGroup; + this.groupsByName = HashMultimap.create(); + for (SecurityGroup groupInRegion : allGroupsInRegion) { + final TenantIdAndName tenantIdAndName = TenantIdAndName.builder() + .tenantId(groupInRegion.getTenantId()) + .name(groupInRegion.getName()) + .build(); + this.groupsByName.put(tenantIdAndName, groupInRegion); + } } public SecurityGroup getSecurityGroup() { return securityGroup; } + /** + * Returns a map of group {@link TenantIdAndName}s to groups. + * + * The returned value is a collection, to take into account the possibility that certain clouds + * may permit duplicate group names. + * + * @return The map of names to (collections of) groups. + */ + public Map> getGroupsByName() { + return groupsByName.asMap(); + } + // superclass hashCode/equals are good enough, and help us use RegionAndName and SecurityGroupInRegion // interchangeably as Map keys @Override protected ToStringHelper string() { - return super.string().add("securityGroup", securityGroup); + return super.string() + .add("securityGroup", securityGroup) + .add("groupsByName", groupsByName); } @Override diff --git a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java index b7b7f0d164..cc1ddd38d8 100644 --- a/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java +++ b/apis/openstack-nova/src/main/java/org/jclouds/openstack/nova/v2_0/predicates/FindSecurityGroupWithNameAndReturnTrue.java @@ -35,6 +35,7 @@ import org.jclouds.rest.ResourceNotFoundException; import com.google.common.base.Optional; import com.google.common.base.Predicate; +import com.google.common.collect.FluentIterable; import com.google.common.collect.Iterables; import com.google.inject.Inject; @@ -63,7 +64,8 @@ public class FindSecurityGroupWithNameAndReturnTrue implements Predicate() { + final FluentIterable allGroups = api.get().list(); + SecurityGroup returnVal = Iterables.find(allGroups, new Predicate() { @Override public boolean apply(SecurityGroup input) { @@ -71,7 +73,7 @@ public class FindSecurityGroupWithNameAndReturnTrue implements Predicate securityGroupExtension = computeService.getSecurityGroupExtension(); + assertTrue(securityGroupExtension.isPresent(), "security extension was not present"); + + logger.info("Loading security groups"); + final SecurityGroupExtension security = securityGroupExtension.get(); + Set beforeAdd = security.listSecurityGroups(); + int countBeforeAdd = beforeAdd.size(); + logger.info("Found %d security groups", countBeforeAdd); + + String someUnlikelyName = String.valueOf(new Random().nextInt(1000000) + 1000000); + logger.info("Adding security group %s", someUnlikelyName); + final SecurityGroup testGroup = security.createSecurityGroup(someUnlikelyName, getNodeTemplate().getLocation()); + + try { + verifyAndDeleteSecurityGroup(security, countBeforeAdd, testGroup); + } catch (Exception e) { + logger.error(e, "Exception caught, live test leaking security group %s", testGroup.getName()); + throw e; + } + + final long end = new Date().getTime(); + + assertTrue(end - begin < TimeUnit.MINUTES.toMillis(5)); // see https://issues.apache.org/jira/browse/JCLOUDS-1235 + + } + + private void verifyAndDeleteSecurityGroup(SecurityGroupExtension security, int countBeforeAdd, + final SecurityGroup testGroup) { + logger.info("Loading security groups"); + Set afterAdd = security.listSecurityGroups(); + final int countAfterAdd = afterAdd.size(); + logger.info("Found %d security groups", countAfterAdd); + + assertEquals(countAfterAdd, countBeforeAdd + 1); + final Predicate findTestGroup = new Predicate() { + @Override + public boolean apply(SecurityGroup input) { + return input.getName().equals(testGroup.getName()); + } + }; + final SecurityGroup created = Iterables.find(afterAdd, findTestGroup); + assertNotNull(created, "Did not find security group created as expected"); + + logger.info("Removing %s", testGroup.getName()); + security.removeSecurityGroup(testGroup.getId()); + + logger.info("Loading security groups"); + Set afterRemove = security.listSecurityGroups(); + final int sizeAfterRemove = afterRemove.size(); + logger.info("Found %d security groups", sizeAfterRemove); + assertEquals(sizeAfterRemove, countBeforeAdd); + final Optional removed = Iterables.tryFind(afterRemove, findTestGroup); + assertTrue(!removed.isPresent(), "Did not remove test security group as expected"); + } } diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroupTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroupTest.java index 969b3f4d5e..ea46cd90cb 100644 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroupTest.java +++ b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupInRegionToSecurityGroupTest.java @@ -16,10 +16,8 @@ */ package org.jclouds.openstack.nova.v2_0.compute.functions; -import static com.google.common.collect.Iterables.transform; -import static org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroupTest.securityGroupWithCidr; -import static org.jclouds.openstack.nova.v2_0.compute.functions.NovaSecurityGroupToSecurityGroupTest.securityGroupWithGroup; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; import java.util.Map; @@ -27,13 +25,19 @@ import org.jclouds.compute.domain.SecurityGroup; import org.jclouds.domain.Location; import org.jclouds.domain.LocationBuilder; import org.jclouds.domain.LocationScope; +import org.jclouds.net.domain.IpPermission; +import org.jclouds.net.domain.IpProtocol; +import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; +import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; import org.testng.annotations.Test; import com.google.common.base.Supplier; import com.google.common.base.Suppliers; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; + @Test(groups = "unit", testName = "NovaSecurityGroupInRegionToSecurityGroupTest") public class NovaSecurityGroupInRegionToSecurityGroupTest { @@ -45,12 +49,67 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest { Supplier> locationIndex = Suppliers.> ofInstance(ImmutableMap .of("az-1.region-a.geo-1", region)); + public static final String SOME_GROUP_ID = "some-group-id"; + public static final String SOME_OTHER_GROUP_ID = "some-other-group-id"; + public static final String IP_RANGE = "0.0.0.0/0"; + public static final String SOME_OTHER_GROUP = "some-other-group"; + public static final String SOME_GROUP = "some-group"; + + public static final ImmutableList allGroups = + ImmutableList.of(securityGroupWithGroup(), securityGroupWithCidr()); + + public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithGroup() { + TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name(SOME_OTHER_GROUP).build(); + + SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() + .id("some-rule-id") + .ipProtocol(IpProtocol.TCP) + .fromPort(10) + .toPort(20) + .group(group) + .parentGroupId(SOME_GROUP_ID) + .build(); + + org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = + org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder() + .tenantId("tenant") + .id(SOME_GROUP_ID) + .name(SOME_GROUP) + .description("some-description") + .rules(ruleToConvert) + .build(); + + return origGroup; + } + + public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithCidr() { + SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() + .id("some-other-rule-id") + .ipProtocol(IpProtocol.TCP) + .fromPort(10) + .toPort(20) + .ipRange(IP_RANGE) + .parentGroupId(SOME_OTHER_GROUP_ID) + .build(); + + org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = + org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder() + .tenantId("tenant") + .id(SOME_OTHER_GROUP_ID) + .name(SOME_OTHER_GROUP) + .description("some-description") + .rules(ruleToConvert) + .build(); + + return origGroup; + } @Test public void testApplyWithGroup() { NovaSecurityGroupInRegionToSecurityGroup parser = createGroupParser(); - SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithGroup(), region.getId()); + final org.jclouds.openstack.nova.v2_0.domain.SecurityGroup otherGroup = securityGroupWithCidr(); + SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithGroup(), region.getId(), allGroups); SecurityGroup newGroup = parser.apply(origGroup); @@ -58,8 +117,11 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest { assertEquals(newGroup.getProviderId(), origGroup.getSecurityGroup().getId()); assertEquals(newGroup.getName(), origGroup.getSecurityGroup().getName()); assertEquals(newGroup.getOwnerId(), origGroup.getSecurityGroup().getTenantId()); - assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getSecurityGroup().getRules(), - NovaSecurityGroupToSecurityGroupTest.ruleConverter))); + final IpPermission permission = Iterables.getOnlyElement(newGroup.getIpPermissions()); + assertEquals(Iterables.getOnlyElement(permission.getGroupIds()), region.getId() + "/" + otherGroup.getId()); + assertEquals(permission.getFromPort(), 10); + assertEquals(permission.getToPort(), 20); + assertTrue(permission.getCidrBlocks().isEmpty()); assertEquals(newGroup.getLocation().getId(), origGroup.getRegion()); } @@ -68,7 +130,7 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest { NovaSecurityGroupInRegionToSecurityGroup parser = createGroupParser(); - SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithCidr(), region.getId()); + SecurityGroupInRegion origGroup = new SecurityGroupInRegion(securityGroupWithCidr(), region.getId(), allGroups); SecurityGroup newGroup = parser.apply(origGroup); @@ -76,15 +138,17 @@ public class NovaSecurityGroupInRegionToSecurityGroupTest { assertEquals(newGroup.getProviderId(), origGroup.getSecurityGroup().getId()); assertEquals(newGroup.getName(), origGroup.getSecurityGroup().getName()); assertEquals(newGroup.getOwnerId(), origGroup.getSecurityGroup().getTenantId()); - assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getSecurityGroup().getRules(), - NovaSecurityGroupToSecurityGroupTest.ruleConverter))); + final IpPermission permission = Iterables.getOnlyElement(newGroup.getIpPermissions()); + assertEquals(permission.getFromPort(), 10); + assertEquals(permission.getToPort(), 20); + assertEquals(Iterables.getOnlyElement(permission.getCidrBlocks()), IP_RANGE); + assertTrue(permission.getGroupIds().isEmpty()); assertEquals(newGroup.getLocation().getId(), origGroup.getRegion()); } private NovaSecurityGroupInRegionToSecurityGroup createGroupParser() { - NovaSecurityGroupToSecurityGroup baseParser = new NovaSecurityGroupToSecurityGroup(NovaSecurityGroupToSecurityGroupTest.ruleConverter); - NovaSecurityGroupInRegionToSecurityGroup parser = new NovaSecurityGroupInRegionToSecurityGroup(baseParser, locationIndex); + NovaSecurityGroupInRegionToSecurityGroup parser = new NovaSecurityGroupInRegionToSecurityGroup(locationIndex); return parser; } diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroupTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroupTest.java deleted file mode 100644 index 5edc44dc1a..0000000000 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/NovaSecurityGroupToSecurityGroupTest.java +++ /dev/null @@ -1,152 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.jclouds.openstack.nova.v2_0.compute.functions; - -import static com.google.common.collect.Iterables.transform; -import static org.testng.Assert.assertEquals; - -import java.util.Map; -import java.util.concurrent.atomic.AtomicReference; - -import org.jclouds.compute.domain.SecurityGroup; -import org.jclouds.domain.Location; -import org.jclouds.domain.LocationBuilder; -import org.jclouds.domain.LocationScope; -import org.jclouds.net.domain.IpProtocol; -import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; -import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName; -import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName; -import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; -import org.testng.annotations.Test; - -import com.google.common.base.Functions; -import com.google.common.base.Predicate; -import com.google.common.base.Predicates; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; -import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableSet; - -@Test(groups = "unit", testName = "NovaSecurityGroupToSecurityGroupTest") -public class NovaSecurityGroupToSecurityGroupTest { - - private static final Location provider = new LocationBuilder().scope(LocationScope.PROVIDER).id("openstack-nova") - .description("openstack-nova").build(); - private static final Location region = new LocationBuilder().id("az-1.region-a.geo-1").description("az-1.region-a.geo-1") - .scope(LocationScope.REGION).parent(provider).build(); - private static final Supplier> locationIndex = Suppliers.> ofInstance(ImmutableMap - .of("az-1.region-a.geo-1", region)); - - - private static final Predicate> returnSecurityGroupExistsInRegion = Predicates.alwaysTrue(); - - private static final Map groupMap = ImmutableMap.of( - RegionAndName.fromRegionAndName("az-1.region-a.geo-1", "some-group"), new SecurityGroupInRegion(securityGroupWithGroup(), "az-1.region-a.geo-1"), - RegionAndName.fromRegionAndName("az-1.region-a.geo-1", "some-other-group"), new SecurityGroupInRegion(securityGroupWithCidr(), "az-1.region-a.geo-1")); - - // weird compilation error means have to declare extra generics for call to build() - see https://bugs.eclipse.org/bugs/show_bug.cgi?id=365818 - private static final Supplier > groupCache = Suppliers.> ofInstance( - CacheBuilder.newBuilder().build(CacheLoader.from(Functions.forMap(groupMap)))); - - public static final SecurityGroupRuleToIpPermission ruleConverter = new SecurityGroupRuleToIpPermission(returnSecurityGroupExistsInRegion, locationIndex, - groupCache.get()); - - public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithGroup() { - TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name("some-other-group").build(); - - SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() - .id("some-id") - .ipProtocol(IpProtocol.TCP) - .fromPort(10) - .toPort(20) - .group(group) - .parentGroupId("some-other-id") - .build(); - - org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder() - .tenantId("tenant") - .id("some-id") - .name("some-group") - .description("some-description") - .rules(ruleToConvert) - .build(); - - return origGroup; - } - - public static org.jclouds.openstack.nova.v2_0.domain.SecurityGroup securityGroupWithCidr() { - SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() - .id("some-id") - .ipProtocol(IpProtocol.TCP) - .fromPort(10) - .toPort(20) - .ipRange("0.0.0.0/0") - .parentGroupId("some-other-id") - .build(); - - org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = org.jclouds.openstack.nova.v2_0.domain.SecurityGroup.builder() - .tenantId("tenant") - .id("some-id") - .name("some-other-group") - .description("some-description") - .rules(ruleToConvert) - .build(); - - return origGroup; - } - - @Test - public void testApplyWithGroup() { - NovaSecurityGroupToSecurityGroup parser = createGroupParser(); - - org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = securityGroupWithGroup(); - - SecurityGroup newGroup = parser.apply(origGroup); - - assertEquals(newGroup.getId(), origGroup.getId()); - assertEquals(newGroup.getProviderId(), origGroup.getId()); - assertEquals(newGroup.getName(), origGroup.getName()); - assertEquals(newGroup.getOwnerId(), origGroup.getTenantId()); - assertEquals(newGroup.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getRules(), ruleConverter))); - } - - @Test - public void testApplyWithCidr() { - - NovaSecurityGroupToSecurityGroup parser = createGroupParser(); - - org.jclouds.openstack.nova.v2_0.domain.SecurityGroup origGroup = securityGroupWithCidr(); - - SecurityGroup group = parser.apply(origGroup); - - assertEquals(group.getId(), origGroup.getId()); - assertEquals(group.getProviderId(), origGroup.getId()); - assertEquals(group.getName(), origGroup.getName()); - assertEquals(group.getOwnerId(), origGroup.getTenantId()); - assertEquals(group.getIpPermissions(), ImmutableSet.copyOf(transform(origGroup.getRules(), ruleConverter))); - } - - private NovaSecurityGroupToSecurityGroup createGroupParser() { - NovaSecurityGroupToSecurityGroup parser = new NovaSecurityGroupToSecurityGroup(ruleConverter); - - return parser; - } - -} diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermissionTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermissionTest.java deleted file mode 100644 index dedf100bfe..0000000000 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/compute/functions/SecurityGroupRuleToIpPermissionTest.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.jclouds.openstack.nova.v2_0.compute.functions; - -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertTrue; - -import org.jclouds.net.domain.IpPermission; -import org.jclouds.net.domain.IpProtocol; -import org.jclouds.openstack.nova.v2_0.domain.SecurityGroupRule; -import org.jclouds.openstack.nova.v2_0.domain.TenantIdAndName; -import org.testng.annotations.Test; - -import com.google.common.collect.ImmutableSet; - - -/** - * Tests for the function for transforming a nova specific SecurityGroupRule into a generic - * IpPermission object. - */ -public class SecurityGroupRuleToIpPermissionTest { - - @Test - public void testApplyWithGroup() { - - TenantIdAndName group = TenantIdAndName.builder().tenantId("tenant").name("some-group").build(); - - SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() - .id("some-id") - .ipProtocol(IpProtocol.TCP) - .fromPort(10) - .toPort(20) - .group(group) - .parentGroupId("some-other-id") - .build(); - - IpPermission convertedPerm = NovaSecurityGroupToSecurityGroupTest.ruleConverter.apply(ruleToConvert); - - assertEquals(convertedPerm.getIpProtocol(), ruleToConvert.getIpProtocol()); - assertEquals(convertedPerm.getFromPort(), ruleToConvert.getFromPort()); - assertEquals(convertedPerm.getToPort(), ruleToConvert.getToPort()); - assertTrue(convertedPerm.getGroupIds().contains("az-1.region-a.geo-1/some-id")); - assertEquals(convertedPerm.getCidrBlocks().size(), 0); - } - - @Test - public void testApplyWithCidr() { - SecurityGroupRule ruleToConvert = SecurityGroupRule.builder() - .id("some-id") - .ipProtocol(IpProtocol.TCP) - .fromPort(10) - .toPort(20) - .ipRange("0.0.0.0/0") - .parentGroupId("some-other-id") - .build(); - - IpPermission convertedPerm = NovaSecurityGroupToSecurityGroupTest.ruleConverter.apply(ruleToConvert); - - assertEquals(convertedPerm.getIpProtocol(), ruleToConvert.getIpProtocol()); - assertEquals(convertedPerm.getFromPort(), ruleToConvert.getFromPort()); - assertEquals(convertedPerm.getToPort(), ruleToConvert.getToPort()); - assertEquals(convertedPerm.getCidrBlocks(), ImmutableSet.of("0.0.0.0/0")); - assertEquals(convertedPerm.getTenantIdGroupNamePairs().size(), 0); - } -} diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java index 3ee404fcd7..248fad88ac 100644 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java +++ b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/CreateSecurityGroupIfNeededTest.java @@ -24,6 +24,7 @@ import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; import org.jclouds.openstack.nova.v2_0.NovaApi; import org.jclouds.openstack.nova.v2_0.compute.functions.CreateSecurityGroupIfNeeded; +import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionSecurityGroupNameAndPorts; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest; @@ -47,13 +48,14 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest { "{\"security_group\":{\"name\":\"jclouds_mygroup\",\"description\":\"jclouds_mygroup\"}}", "application/json")).build(); + private final int groupId = 2769; + public void testCreateNewGroup() throws Exception { Builder builder = ImmutableMap.builder(); builder.put(keystoneAuthWithUsernameAndPasswordAndTenantName, responseWithKeystoneAccess); builder.put(extensionsOfNovaRequest, extensionsOfNovaResponse); - int groupId = 2769; HttpResponse createResponse = HttpResponse.builder().statusCode(200) .payload( @@ -113,15 +115,24 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest { builder.put(getSecurityGroup, getSecurityGroupResponse); + + HttpRequest listSecurityGroups = HttpRequest.builder().method("GET").endpoint( + URI.create("https://az-1.region-a.geo-1.compute.hpcloudsvc.com/v2/3456/os-security-groups")).headers( + ImmutableMultimap. builder().put("Accept", "application/json").put("X-Auth-Token", + authToken).build()).build(); + HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200).payload( + payloadFromResource("/securitygroup_list_details_computeservice_typical.json")).build(); + builder.put(listSecurityGroups, listSecurityGroupsResponse); + NovaApi apiCanCreateSecurityGroup = requestsSendResponses(builder.build()); CreateSecurityGroupIfNeeded fn = new CreateSecurityGroupIfNeeded(apiCanCreateSecurityGroup); // we can find it - assertEquals(fn.apply( - new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))) - .toString(), new SecurityGroupInRegion(new ParseComputeServiceTypicalSecurityGroupTest().expected(), - "az-1.region-a.geo-1").toString()); + final SecurityGroup expected = new ParseComputeServiceTypicalSecurityGroupTest().expected(); + assertEquals( + fn.apply(new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))).toString(), + new SecurityGroupInRegion(expected, "az-1.region-a.geo-1", ImmutableList.of(expected)).toString()); } @@ -155,10 +166,9 @@ public class CreateSecurityGroupIfNeededTest extends BaseNovaApiExpectTest { CreateSecurityGroupIfNeeded fn = new CreateSecurityGroupIfNeeded(apiWhenSecurityGroupsExist); // we can find it - assertEquals(fn.apply( - new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))) - .toString(), new SecurityGroupInRegion(new ParseComputeServiceTypicalSecurityGroupTest().expected(), - "az-1.region-a.geo-1").toString()); - + final SecurityGroup expected = new ParseComputeServiceTypicalSecurityGroupTest().expected(); + assertEquals( + fn.apply(new RegionSecurityGroupNameAndPorts("az-1.region-a.geo-1", "jclouds_mygroup", ImmutableSet.of(22, 8080))).toString(), + new SecurityGroupInRegion(expected, "az-1.region-a.geo-1", ImmutableList.of(expected)).toString()); } } diff --git a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/FindSecurityGroupWithNameAndReturnTrueExpectTest.java b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/FindSecurityGroupWithNameAndReturnTrueExpectTest.java index d49fb8c1e5..e9e9b4d077 100644 --- a/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/FindSecurityGroupWithNameAndReturnTrueExpectTest.java +++ b/apis/openstack-nova/src/test/java/org/jclouds/openstack/nova/v2_0/functions/FindSecurityGroupWithNameAndReturnTrueExpectTest.java @@ -21,11 +21,13 @@ import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; import java.net.URI; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import org.jclouds.http.HttpRequest; import org.jclouds.http.HttpResponse; import org.jclouds.openstack.nova.v2_0.NovaApi; +import org.jclouds.openstack.nova.v2_0.domain.SecurityGroup; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.RegionAndName; import org.jclouds.openstack.nova.v2_0.domain.regionscoped.SecurityGroupInRegion; import org.jclouds.openstack.nova.v2_0.internal.BaseNovaApiExpectTest; @@ -63,9 +65,10 @@ public class FindSecurityGroupWithNameAndReturnTrueExpectTest extends BaseNovaAp assertTrue(predicate.apply(securityGroupInRegionRef)); // the reference is now up to date, and includes the actual group found. - assertEquals(securityGroupInRegionRef.get().toString(), new SecurityGroupInRegion(Iterables - .getOnlyElement(new ParseSecurityGroupListTest().expected()), "az-1.region-a.geo-1").toString()); - + final Set expected = new ParseSecurityGroupListTest().expected(); + assertEquals( + securityGroupInRegionRef.get().toString(), + new SecurityGroupInRegion(Iterables.getOnlyElement(expected), "az-1.region-a.geo-1", expected).toString()); } public void testDoesNotUpdateReferenceWhenSecurityGroupListMissingGroupName() throws Exception { diff --git a/core/src/test/java/org/jclouds/rest/internal/BaseRestApiExpectTest.java b/core/src/test/java/org/jclouds/rest/internal/BaseRestApiExpectTest.java index 6b4e2ec845..d0f1aa4eff 100644 --- a/core/src/test/java/org/jclouds/rest/internal/BaseRestApiExpectTest.java +++ b/core/src/test/java/org/jclouds/rest/internal/BaseRestApiExpectTest.java @@ -337,7 +337,8 @@ public abstract class BaseRestApiExpectTest { String.format("request %s is out of range (%s)", index, requests.size())).payload( Payloads.newStringPayload(renderRequest(input))).build(); if (!httpRequestsAreEqual(input, requests.get(index))) { - assertEquals(renderRequest(input), renderRequest(requests.get(index))); + assertEquals(renderRequest(input), renderRequest(requests.get(index)), + "Actual request did not match expected request " + index); } return responses.get(index); }