You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by sv...@apache.org on 2016/10/31 13:34:25 UTC
[2/3] brooklyn-server git commit: Clean up duplicated code in
JcloudsLocationCustomizers
Clean up duplicated code in JcloudsLocationCustomizers
Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo
Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/53bbcab0
Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/53bbcab0
Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/53bbcab0
Branch: refs/heads/master
Commit: 53bbcab0c5ab615420a697001579675fb0bd0fb3
Parents: 44d11e2
Author: Valentin Aitken <bo...@gmail.com>
Authored: Mon Sep 5 13:00:48 2016 +0300
Committer: Valentin Aitken <bo...@gmail.com>
Committed: Thu Oct 20 11:30:59 2016 +0100
----------------------------------------------------------------------
.../InboundPortsJcloudsLocationCustomizer.java | 4 ++
.../JcloudsLocationSecurityGroupCustomizer.java | 17 ++++---
.../jclouds/networking/NetworkingEffectors.java | 51 ++++++++------------
.../SharedLocationSecurityGroupCustomizer.java | 40 +++++++++++++--
...oudsLocationSecurityGroupCustomizerTest.java | 24 ++++++++-
...aredLocationSecurityGroupCustomizerTest.java | 8 +++
.../location/NetworkingEffectorsLiveTests.java | 45 +++++++++++------
7 files changed, 132 insertions(+), 57 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/InboundPortsJcloudsLocationCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/InboundPortsJcloudsLocationCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/InboundPortsJcloudsLocationCustomizer.java
index 11b01ee..4fc3d11 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/InboundPortsJcloudsLocationCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/InboundPortsJcloudsLocationCustomizer.java
@@ -35,7 +35,11 @@ import org.slf4j.LoggerFactory;
import static org.apache.brooklyn.location.jclouds.networking.NetworkingEffectors.*;
+/**
+ * @deprecated Please use {@link org.apache.brooklyn.location.jclouds.networking.SharedLocationSecurityGroupCustomizer}
+ */
@Beta
+@Deprecated
public class InboundPortsJcloudsLocationCustomizer extends BasicJcloudsLocationCustomizer {
public static final Logger LOG = LoggerFactory.getLogger(InboundPortsJcloudsLocationCustomizer.class);
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
index e15ad68..63fb020 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java
@@ -22,12 +22,14 @@ import static com.google.common.base.Preconditions.checkNotNull;
import java.util.Collection;
import java.util.Iterator;
+import java.util.Map;
import java.util.Set;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutionException;
import javax.annotation.Nullable;
+import org.apache.brooklyn.util.collections.MutableMap;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -45,6 +47,7 @@ import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.FluentIterable;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
import com.google.common.util.concurrent.UncheckedExecutionException;
@@ -67,7 +70,6 @@ import org.apache.brooklyn.location.jclouds.JcloudsLocation;
import org.apache.brooklyn.location.jclouds.JcloudsLocationConfig;
import org.apache.brooklyn.location.jclouds.JcloudsLocationCustomizer;
import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
-import org.apache.brooklyn.location.jclouds.JcloudsSshMachineLocation;
import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.core.task.Tasks;
import org.apache.brooklyn.util.exceptions.Exceptions;
@@ -168,13 +170,13 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
return this;
}
- /** @see #addPermissionsToLocation(JcloudsSshMachineLocation, java.lang.Iterable) */
+ /** @see #addPermissionsToLocation(JcloudsMachineLocation, java.lang.Iterable) */
public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsMachineLocation location, IpPermission... permissions) {
addPermissionsToLocation(location, ImmutableList.copyOf(permissions));
return this;
}
- /** @see #addPermissionsToLocation(JcloudsSshMachineLocation, java.lang.Iterable) */
+ /** @see #addPermissionsToLocation(JcloudsMachineLocation, java.lang.Iterable) */
public JcloudsLocationSecurityGroupCustomizer addPermissionsToLocation(final JcloudsMachineLocation location, SecurityGroupDefinition securityGroupDefinition) {
addPermissionsToLocation(location, securityGroupDefinition.getPermissions());
return this;
@@ -280,11 +282,11 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
* @param computeService The compute service to use to apply the changes
*/
@VisibleForTesting
- void addPermissionsToLocation(Iterable<IpPermission> permissions, final String nodeId, ComputeService computeService) {
+ Map<String, SecurityGroup> addPermissionsToLocation(Iterable<IpPermission> permissions, final String nodeId, ComputeService computeService) {
if (!computeService.getSecurityGroupExtension().isPresent()) {
LOG.warn("Security group extension for {} absent; cannot update node {} with {}",
new Object[] {computeService, nodeId, permissions});
- return;
+ return ImmutableMap.of();
}
final SecurityGroupExtension securityApi = computeService.getSecurityGroupExtension().get();
final String locationId = computeService.getContext().unwrap().getId();
@@ -296,9 +298,12 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation
SecurityGroup machineUniqueSecurityGroup = getSecurityGroup(nodeId, securityApi, locationId);
MutableList<IpPermission> newPermissions = MutableList.copyOf(permissions);
Iterables.removeAll(newPermissions, machineUniqueSecurityGroup.getIpPermissions());
+ MutableMap<String, SecurityGroup> addedSecurityGroups = MutableMap.of();
for (IpPermission permission : newPermissions) {
- addPermission(permission, machineUniqueSecurityGroup, securityApi);
+ SecurityGroup addedPermission = addPermission(permission, machineUniqueSecurityGroup, securityApi);
+ addedSecurityGroups.put(addedPermission.getId(), addedPermission);
}
+ return addedSecurityGroups;
}
/**
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/NetworkingEffectors.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/NetworkingEffectors.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/NetworkingEffectors.java
index aba9753..876e059 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/NetworkingEffectors.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/NetworkingEffectors.java
@@ -22,7 +22,6 @@ import com.google.common.annotations.Beta;
import com.google.common.base.Optional;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Range;
import com.google.common.reflect.TypeToken;
import org.apache.brooklyn.api.effector.Effector;
import org.apache.brooklyn.api.location.Location;
@@ -30,14 +29,14 @@ import org.apache.brooklyn.config.ConfigKey;
import org.apache.brooklyn.core.config.ConfigKeys;
import org.apache.brooklyn.core.effector.EffectorBody;
import org.apache.brooklyn.core.effector.Effectors;
+import org.apache.brooklyn.location.jclouds.JcloudsLocation;
import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
-import org.apache.brooklyn.util.collections.MutableList;
import org.apache.brooklyn.util.core.config.ConfigBag;
-import org.apache.brooklyn.util.net.Cidr;
-import org.apache.brooklyn.util.net.Networking;
+import org.jclouds.compute.domain.SecurityGroup;
import org.jclouds.net.domain.IpPermission;
import org.jclouds.net.domain.IpProtocol;
+import java.util.Collection;
import java.util.List;
import static com.google.common.base.Predicates.instanceOf;
@@ -49,7 +48,7 @@ public class NetworkingEffectors {
public static final ConfigKey<List<String>> INBOUND_PORTS_LIST = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "inbound.ports.list",
"Ports to open from the effector", ImmutableList.<String>of());
public static final ConfigKey<IpProtocol> INBOUND_PORTS_LIST_PROTOCOL = ConfigKeys.newConfigKey(new TypeToken<IpProtocol>() {}, "inbound.ports.list.protocol",
- "Protocol for ports to open. Possible values: TCP, UDP, ICMP, ALL.", IpProtocol.TCP);
+ "Protocol for ports to open. Possible values: TCP, UDP, ICMP.", IpProtocol.TCP);
public static final ConfigKey<JcloudsMachineLocation> JCLOUDS_MACHINE_LOCATIN = ConfigKeys.newConfigKey(JcloudsMachineLocation.class, "jcloudsMachineLocation");
@@ -65,38 +64,30 @@ public class NetworkingEffectors {
@SuppressWarnings("rawtypes")
private static class OpenPortsInSecurityGroupBody extends EffectorBody<Iterable> {
@Override
- public Iterable<IpPermission> call(ConfigBag parameters) {
+ public Collection<SecurityGroup> call(ConfigBag parameters) {
List<String> rawPortRules = parameters.get(INBOUND_PORTS_LIST);
IpProtocol ipProtocol = parameters.get(INBOUND_PORTS_LIST_PROTOCOL);
- JcloudsMachineLocation jcloudsMachineLocation = parameters.get(JCLOUDS_MACHINE_LOCATIN);
Preconditions.checkNotNull(ipProtocol, INBOUND_PORTS_LIST_PROTOCOL.getName() + " cannot be null");
Preconditions.checkNotNull(rawPortRules, INBOUND_PORTS_LIST.getName() + " cannot be null");
- MutableList.Builder<IpPermission> ipPermissionsBuilder = MutableList.builder();
- for (Range<Integer> portRule : Networking.portRulesToRanges(rawPortRules).asRanges()) {
- ipPermissionsBuilder.add(
- IpPermission.builder()
- .ipProtocol(ipProtocol)
- .fromPort(portRule.lowerEndpoint())
- .toPort(portRule.upperEndpoint())
- .cidrBlock(Cidr.UNIVERSAL.toString())
- .build());
+
+ SharedLocationSecurityGroupCustomizer locationSecurityGroupCustomizer = new SharedLocationSecurityGroupCustomizer();
+ if (IpProtocol.TCP.equals(ipProtocol)) {
+ locationSecurityGroupCustomizer.setTcpPortRanges(rawPortRules);
+ } else if (IpProtocol.UDP.equals(ipProtocol)) {
+ locationSecurityGroupCustomizer.setUdpPortRanges(rawPortRules);
+ } else if (IpProtocol.ICMP.equals(ipProtocol)) {
+ locationSecurityGroupCustomizer.setOpenIcmp(true);
}
- JcloudsLocationSecurityGroupCustomizer customizer = JcloudsLocationSecurityGroupCustomizer.getInstance(entity());
- if (jcloudsMachineLocation == null) {
- Optional<Location> jcloudsMachineLocationOptional = tryFind(
- (Iterable<Location>) getLocationsCheckingAncestors(null, entity()),
- instanceOf(JcloudsMachineLocation.class));
- if (!jcloudsMachineLocationOptional.isPresent()) {
- throw new IllegalArgumentException("Tried to execute open ports effector on an entity with no JcloudsMachineLocation");
- } else {
- jcloudsMachineLocation = (JcloudsMachineLocation)jcloudsMachineLocationOptional.get();
- }
+ Optional<Location> jcloudsMachineLocationOptional = tryFind(
+ (Iterable<Location>) getLocationsCheckingAncestors(null, entity()),
+ instanceOf(JcloudsMachineLocation.class));
+ if (!jcloudsMachineLocationOptional.isPresent()) {
+ throw new IllegalArgumentException("Tried to execute open ports effector on an entity with no JcloudsMachineLocation");
}
- Iterable<IpPermission> ipPermissionsToAdd = ipPermissionsBuilder.build();
- customizer.addPermissionsToLocation(jcloudsMachineLocation, ipPermissionsToAdd);
- return ipPermissionsToAdd;
+ JcloudsLocation jcloudsLocation = ((JcloudsMachineLocation)jcloudsMachineLocationOptional.get()).getParent();
+
+ return locationSecurityGroupCustomizer.applySecurityGroupCustomizations(jcloudsLocation, jcloudsLocation.getComputeService(),(JcloudsMachineLocation)jcloudsMachineLocationOptional.get());
}
}
-
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java
index 6dc7906..1327218 100644
--- a/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java
+++ b/locations/jclouds/src/main/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizer.java
@@ -18,6 +18,7 @@
*/
package org.apache.brooklyn.location.jclouds.networking;
+import java.util.Collection;
import java.util.List;
import javax.annotation.Nullable;
@@ -28,6 +29,7 @@ import org.apache.brooklyn.location.jclouds.JcloudsMachineLocation;
import org.apache.brooklyn.util.net.Cidr;
import org.apache.brooklyn.util.net.Networking;
import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.SecurityGroup;
import org.jclouds.compute.domain.Template;
import org.jclouds.net.domain.IpPermission;
import org.jclouds.net.domain.IpProtocol;
@@ -76,6 +78,12 @@ public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationC
private RangeSet<Integer> udpPortRanges;
/**
+ * Tested only on AWS only.
+ * It depends on the cloud provider and jclouds driver whether security group allows opening ICMP.
+ */
+ private Boolean openIcmp;
+
+ /**
* Add a flag that disables this customizer. It's isn't currently possible to add a customizer
* based on a flag. This flag makes it possible to write blueprints using the customizer but still
* be able to disable it for clouds (e.g. bluebox) where the SG implementation has known issues.
@@ -106,6 +114,10 @@ public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationC
this.udpPortRanges = Networking.portRulesToRanges(udpPortRanges);
}
+ public void setOpenIcmp(Boolean openIcmp) {
+ this.openIcmp = openIcmp;
+ }
+
/**
* Set to restict the range that the ports that are to be opened can be accessed from
*
@@ -137,15 +149,27 @@ public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationC
@Override
public void customize(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
+ applySecurityGroupCustomizations(location, computeService, machine);
+ }
+
+ public Collection<SecurityGroup> applySecurityGroupCustomizations(JcloudsLocation location, ComputeService computeService, JcloudsMachineLocation machine) {
super.customize(location, computeService, machine);
- if(!enabled) return;
+ if(!enabled) return ImmutableList.of();
final JcloudsLocationSecurityGroupCustomizer instance = getInstance(getSharedGroupId(location));
ImmutableList.Builder<IpPermission> builder = ImmutableList.<IpPermission>builder();
+
builder.addAll(getIpPermissions(instance, tcpPortRanges, IpProtocol.TCP));
builder.addAll(getIpPermissions(instance, udpPortRanges, IpProtocol.UDP));
+ if (Boolean.TRUE.equals(openIcmp)) {
+ builder.addAll(ImmutableList.of(
+ IpPermission
+ .builder().ipProtocol(IpProtocol.ICMP).fromPort(-1).toPort(-1)
+ .cidrBlock(instance.getBrooklynCidrBlock())
+ .build()));
+ }
if (inboundPorts != null) {
for (int inboundPort : inboundPorts) {
@@ -158,7 +182,16 @@ public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationC
builder.add(ipPermission);
}
}
- instance.addPermissionsToLocation(machine, builder.build());
+
+ /**
+ * Same as
+ * {@link JcloudsLocationSecurityGroupCustomizer#addPermissionsToLocation(final JcloudsMachineLocation location, final Iterable<IpPermission>)}
+ * but with different return type.
+ */
+ synchronized (JcloudsLocationSecurityGroupCustomizer.class) {
+ String nodeId = machine.getNode().getId();
+ return instance.addPermissionsToLocation(builder.build(), nodeId, computeService).values();
+ }
}
private List<IpPermission> getIpPermissions(JcloudsLocationSecurityGroupCustomizer instance, RangeSet<Integer> portRanges, IpProtocol protocol) {
@@ -200,6 +233,3 @@ public class SharedLocationSecurityGroupCustomizer extends BasicJcloudsLocationC
return JcloudsLocationSecurityGroupCustomizer.getInstance(sharedGroupId);
}
}
-
-
-
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
index e88ef21..afc722a 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java
@@ -34,6 +34,7 @@ import static org.testng.Assert.assertTrue;
import java.net.URI;
import java.util.Collections;
+import java.util.Set;
import org.jclouds.aws.AWSResponseException;
import org.jclouds.aws.domain.AWSError;
@@ -139,6 +140,9 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
SecurityGroup group = newGroup("id");
when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group));
+ SecurityGroup updatedSecurityGroup = newGroup("id", ImmutableSet.of(ssh, jmx));
+ when(securityApi.addIpPermission(ssh, group)).thenReturn(updatedSecurityGroup);
+ when(securityApi.addIpPermission(jmx, group)).thenReturn(updatedSecurityGroup);
when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
@@ -156,6 +160,9 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
SecurityGroup group = newGroup("id");
when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group));
+ SecurityGroup updatedSecurityGroup = newGroup("id", ImmutableSet.of(ssh, jmx));
+ when(securityApi.addIpPermission(ssh, group)).thenReturn(updatedSecurityGroup);
+ when(securityApi.addIpPermission(jmx, group)).thenReturn(updatedSecurityGroup);
when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
@@ -173,6 +180,9 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup());
SecurityGroup group = newGroup("id");
when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group));
+ SecurityGroup updatedSecurityGroup = newGroup("id", ImmutableSet.of(ssh, jmx));
+ when(securityApi.addIpPermission(ssh, group)).thenReturn(updatedSecurityGroup);
+ when(securityApi.addIpPermission(jmx, group)).thenReturn(updatedSecurityGroup);
when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService);
@@ -220,6 +230,10 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
customizer.customize(jcloudsLocation, computeService, template);
reset(securityApi);
when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(uniqueGroup, sharedGroup));
+ SecurityGroup updatedSharedSecurityGroup = newGroup(sharedGroup.getId(), ImmutableSet.of(ssh));
+ when(securityApi.addIpPermission(ssh, uniqueGroup)).thenReturn(updatedSharedSecurityGroup);
+ SecurityGroup updatedUniqueSecurityGroup = newGroup("unique", ImmutableSet.of(ssh));
+ when(securityApi.addIpPermission(ssh, sharedGroup)).thenReturn(updatedUniqueSecurityGroup);
customizer.addPermissionsToLocation(ImmutableSet.of(ssh), nodeId, computeService);
// Expect the per-machine group to have been altered, not the shared group
@@ -236,6 +250,10 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup));
when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2");
+ SecurityGroup updatedSecurityGroup = newGroup(uniqueGroup.getId(), ImmutableSet.of(ssh));
+ when(securityApi.addIpPermission(ssh, sharedGroup)).thenReturn(updatedSecurityGroup);
+ SecurityGroup updatedUniqueSecurityGroup = newGroup(uniqueGroup.getId(), ImmutableSet.of(ssh));
+ when(securityApi.addIpPermission(ssh, updatedUniqueSecurityGroup)).thenReturn(updatedUniqueSecurityGroup);
// Expect first call to list security groups on nodeId, second to use cached version
customizer.addPermissionsToLocation(ImmutableSet.of(ssh), nodeId, computeService);
@@ -330,6 +348,10 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
}
private SecurityGroup newGroup(String id) {
+ return newGroup(id, ImmutableSet.<IpPermission>of());
+ }
+
+ private SecurityGroup newGroup(String id, Set<IpPermission> ipPermissions) {
URI uri = null;
String ownerId = null;
return new SecurityGroup(
@@ -340,7 +362,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest {
uri,
Collections.<String, String>emptyMap(),
ImmutableSet.<String>of(),
- ImmutableSet.<IpPermission>of(),
+ ipPermissions,
ownerId);
}
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java
----------------------------------------------------------------------
diff --git a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java
index 7c7de4b..880e4ff 100644
--- a/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java
+++ b/locations/jclouds/src/test/java/org/apache/brooklyn/location/jclouds/networking/SharedLocationSecurityGroupCustomizerTest.java
@@ -111,6 +111,14 @@ public class SharedLocationSecurityGroupCustomizerTest {
}
@Test
+ public void testInboundIcmpAddedToPermissions() {
+ customizer.setOpenIcmp(true);
+ when(sgCustomizer.getBrooklynCidrBlock()).thenReturn(Cidr.UNIVERSAL.toString());
+ customizer.customize(jcloudsLocation, computeService, mock(JcloudsMachineLocation.class));
+ assertPermissionsAdded(-1, -1, IpProtocol.ICMP);
+ }
+
+ @Test
public void testInboundPortsAddedToPermissions() {
when(mockOptions.getInboundPorts()).thenReturn(new int[]{5});
when(sgCustomizer.getBrooklynCidrBlock()).thenReturn("10.10.10.10/24");
http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/53bbcab0/software/base/src/test/java/org/apache/brooklyn/entity/software/base/location/NetworkingEffectorsLiveTests.java
----------------------------------------------------------------------
diff --git a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/location/NetworkingEffectorsLiveTests.java b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/location/NetworkingEffectorsLiveTests.java
index 482432c..de0c324 100644
--- a/software/base/src/test/java/org/apache/brooklyn/entity/software/base/location/NetworkingEffectorsLiveTests.java
+++ b/software/base/src/test/java/org/apache/brooklyn/entity/software/base/location/NetworkingEffectorsLiveTests.java
@@ -41,12 +41,14 @@ import org.jclouds.net.domain.IpPermission;
import org.jclouds.net.domain.IpProtocol;
import org.testng.annotations.Test;
+import java.util.Collection;
import java.util.Map;
import java.util.Set;
import static org.apache.brooklyn.location.jclouds.networking.NetworkingEffectors.INBOUND_PORTS_LIST;
import static org.apache.brooklyn.location.jclouds.networking.NetworkingEffectors.INBOUND_PORTS_LIST_PROTOCOL;
import static org.apache.brooklyn.test.Asserts.assertTrue;
+import static org.jclouds.net.domain.IpProtocol.ICMP;
import static org.jclouds.net.domain.IpProtocol.TCP;
import static org.jclouds.net.domain.IpProtocol.UDP;
@@ -67,37 +69,50 @@ public abstract class NetworkingEffectorsLiveTests extends BrooklynAppLiveTestSu
String nodeId = ((JcloudsMachineLocation)jcloudsMachineLocation.get()).getNode().getId();
final SecurityGroupExtension securityApi = computeService.getSecurityGroupExtension().get();
- Effector<Iterable<IpPermission>> openPortsInSecurityGroup = (Effector<Iterable<IpPermission>>)EffectorUtils.findEffectorDeclared(emptySoftwareProcess, "openPortsInSecurityGroup").get();
- Task<Iterable<IpPermission>> task = EffectorUtils.invokeEffectorAsync(emptySoftwareProcess, openPortsInSecurityGroup,
+ Effector<Collection<SecurityGroup>> openPortsInSecurityGroup = (Effector<Collection<SecurityGroup>>)EffectorUtils.findEffectorDeclared(emptySoftwareProcess, "openPortsInSecurityGroup").get();
+ Task<Collection<SecurityGroup>> task = EffectorUtils.invokeEffectorAsync(emptySoftwareProcess, openPortsInSecurityGroup,
ImmutableMap.of(INBOUND_PORTS_LIST.getName(), "234,324,550-1050"));
- Iterable<IpPermission> effectorResult = task.getUnchecked();
- for (Predicate<IpPermission> ipPermissionPredicate : ImmutableList.of(ruleExistsPredicate(234, 234, TCP), ruleExistsPredicate(324, 324, TCP), ruleExistsPredicate(550, 1050, TCP))) {
+ Collection<SecurityGroup> effectorResult = task.getUnchecked();
+ for (Predicate<SecurityGroup> ipPermissionPredicate : ImmutableList.of(ruleExistsPredicate(234, 234, TCP), ruleExistsPredicate(324, 324, TCP), ruleExistsPredicate(550, 1050, TCP))) {
assertTrue(Iterables.tryFind(effectorResult, ipPermissionPredicate).isPresent());
}
task = EffectorUtils.invokeEffectorAsync(emptySoftwareProcess, openPortsInSecurityGroup,
ImmutableMap.of(INBOUND_PORTS_LIST.getName(), "234,324,550-1050", INBOUND_PORTS_LIST_PROTOCOL.getName(), "UDP"));
effectorResult = task.getUnchecked();
- for (Predicate<IpPermission> ipPermissionPredicate : ImmutableList.of(ruleExistsPredicate(234, 234, UDP), ruleExistsPredicate(324, 324, UDP), ruleExistsPredicate(550, 1050, UDP))) {
+ for (Predicate<SecurityGroup> ipPermissionPredicate : ImmutableList.of(ruleExistsPredicate(234, 234, UDP), ruleExistsPredicate(324, 324, UDP), ruleExistsPredicate(550, 1050, UDP))) {
assertTrue(Iterables.tryFind(effectorResult, ipPermissionPredicate).isPresent());
}
Set<SecurityGroup> groupsOnNode = securityApi.listSecurityGroupsForNode(nodeId);
SecurityGroup securityGroup = Iterables.getOnlyElement(groupsOnNode);
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(234, 234, TCP)).isPresent());
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(324, 324, TCP)).isPresent());
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(550, 1050, TCP)).isPresent());
+ assertTrue(ruleExistsPredicate(234, 234, TCP).apply(securityGroup));
+ assertTrue(ruleExistsPredicate(324, 324, TCP).apply(securityGroup));
+ assertTrue(ruleExistsPredicate(550, 1050, TCP).apply(securityGroup));
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(234, 234, UDP)).isPresent());
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(324, 324, UDP)).isPresent());
- assertTrue(Iterables.tryFind(securityGroup.getIpPermissions(), ruleExistsPredicate(550, 1050, UDP)).isPresent());
+ assertTrue(ruleExistsPredicate(234, 234, UDP).apply(securityGroup));
+ assertTrue(ruleExistsPredicate(324, 324, UDP).apply(securityGroup));
+ assertTrue(ruleExistsPredicate(550, 1050, UDP).apply(securityGroup));
+
+ task = EffectorUtils.invokeEffectorAsync(emptySoftwareProcess, openPortsInSecurityGroup,
+ ImmutableMap.of(INBOUND_PORTS_LIST.getName(), "-1", INBOUND_PORTS_LIST_PROTOCOL.getName(), "ICMP"));
+ effectorResult = task.getUnchecked();
+ assertTrue(Iterables.tryFind(effectorResult, ruleExistsPredicate(-1, -1, ICMP)).isPresent());
+ groupsOnNode = securityApi.listSecurityGroupsForNode(nodeId);
+ securityGroup = Iterables.getOnlyElement(groupsOnNode);
+ assertTrue(ruleExistsPredicate(-1, -1, ICMP).apply(securityGroup));
}
- protected Predicate<IpPermission> ruleExistsPredicate(final int fromPort, final int toPort, final IpProtocol ipProtocol) {
- return new Predicate<IpPermission>() {
- public boolean apply(IpPermission ipPermission) {
- return ipPermission.getFromPort() == fromPort && ipPermission.getToPort() == toPort && ipPermission.getIpProtocol() == ipProtocol;
+ protected Predicate<SecurityGroup> ruleExistsPredicate(final int fromPort, final int toPort, final IpProtocol ipProtocol) {
+ return new Predicate<SecurityGroup>() {
+ public boolean apply(SecurityGroup scipPermission) {
+ for (IpPermission ipPermission : scipPermission.getIpPermissions()) {
+ if (ipPermission.getFromPort() == fromPort && ipPermission.getToPort() == toPort && ipPermission.getIpProtocol() == ipProtocol) {
+ return true;
+ }
+ }
+ return false;
}
};
}