Repository: incubator-brooklyn Updated Branches: refs/heads/master b97370b14 -> e6235d9bc
Ported some Clocker changes back to brooklyn. It is now possible to remove security groups permissions Also added some error handling to JcloudsLocationSecurityGroupCustomizer Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/a4de7439 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/a4de7439 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/a4de7439 Branch: refs/heads/master Commit: a4de7439b51ff5b98340c4ecf39c78934b930b77 Parents: b39ef3a Author: Graeme-Miller <graeme.mil...@cloudsoftcorp.com> Authored: Thu Dec 10 15:37:01 2015 +0000 Committer: Graeme-Miller <graeme.mil...@cloudsoftcorp.com> Committed: Tue Dec 15 11:32:03 2015 +0000 ---------------------------------------------------------------------- .../JcloudsLocationSecurityGroupCustomizer.java | 103 ++++++++++++++++--- ...oudsLocationSecurityGroupCustomizerTest.java | 55 ++++++++++ 2 files changed, 141 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/a4de7439/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 8ab6c16..3d6bc22 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 @@ -72,6 +72,7 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; +import com.google.common.util.concurrent.UncheckedExecutionException; /** * Configures custom security groups on Jclouds locations. @@ -179,6 +180,33 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation return this; } + private SecurityGroup getSecurityGroup(final String nodeId, final SecurityGroupExtension securityApi, final String locationId) { + // Expect to have two security groups on the node: one shared between all nodes in the location, + // that is cached in sharedGroupCache, and one created by Jclouds that is unique to the node. + // Relies on customize having been called before. This should be safe because the arguments + // needed to call this method are not available until post-instance creation. + SecurityGroup machineUniqueSecurityGroup; + Tasks.setBlockingDetails("Loading unique security group for node: " + nodeId); + try { + machineUniqueSecurityGroup = uniqueGroupCache.get(nodeId, new Callable<SecurityGroup>() { + @Override public SecurityGroup call() throws Exception { + SecurityGroup sg = getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(nodeId, locationId, securityApi); + if (sg == null) { + throw new IllegalStateException("Failed to find machine-unique group on node: " + nodeId); + } + return sg; + } + }); + } catch (UncheckedExecutionException e) { + throw Throwables.propagate(new Exception(e.getCause())); + } catch (ExecutionException e) { + throw Throwables.propagate(new Exception(e.getCause())); + } finally { + Tasks.resetBlockingDetails(); + } + return machineUniqueSecurityGroup; + } + /** * Applies the given security group permissions to the given location. * <p> @@ -201,6 +229,47 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation return this; } } + /** + * Removes the given security group permissions from the given node with the given compute service. + * <p> + * Takes no action if the compute service does not have a security group extension. + * @param permissions The set of permissions to be removed from the location + * @param location Location to remove permissions from + */ + public void removePermissionsFromLocation(final JcloudsMachineLocation location, final Iterable<IpPermission> permissions) { + synchronized (JcloudsLocationSecurityGroupCustomizer.class) { + ComputeService computeService = location.getParent().getComputeService(); + String nodeId = location.getNode().getId(); + removePermissionsFromLocation(permissions, nodeId, computeService); + } + } + + /** + * Removes the given security group permissions from the given node with the given compute service. + * <p> + * Takes no action if the compute service does not have a security group extension. + * @param permissions The set of permissions to be removed from the node + * @param nodeId The id of the node to update + * @param computeService The compute service to use to apply the changes + */ + @VisibleForTesting + void removePermissionsFromLocation(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; + } + + final SecurityGroupExtension securityApi = computeService.getSecurityGroupExtension().get(); + final String locationId = computeService.getContext().unwrap().getId(); + SecurityGroup machineUniqueSecurityGroup = getSecurityGroup(nodeId, securityApi, locationId); + + for (IpPermission permission : permissions) { + removePermission(permission, machineUniqueSecurityGroup, securityApi); + } + } + + /** * Applies the given security group permissions to the given node with the given compute service. @@ -224,23 +293,7 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation // that is cached in sharedGroupCache, and one created by Jclouds that is unique to the node. // Relies on customize having been called before. This should be safe because the arguments // needed to call this method are not available until post-instance creation. - SecurityGroup machineUniqueSecurityGroup; - Tasks.setBlockingDetails("Loading unique security group for node: " + nodeId); - try { - machineUniqueSecurityGroup = uniqueGroupCache.get(nodeId, new Callable<SecurityGroup>() { - @Override public SecurityGroup call() throws Exception { - SecurityGroup sg = getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(nodeId, locationId, securityApi); - if (sg == null) { - throw new IllegalStateException("Failed to find machine-unique group on node: " + nodeId); - } - return sg; - } - }); - } catch (ExecutionException e) { - throw Throwables.propagate(new Exception(e.getCause())); - } finally { - Tasks.resetBlockingDetails(); - } + SecurityGroup machineUniqueSecurityGroup = getSecurityGroup(nodeId, securityApi, locationId); MutableList<IpPermission> newPermissions = MutableList.copyOf(permissions); Iterables.removeAll(newPermissions, machineUniqueSecurityGroup.getIpPermissions()); for (IpPermission permission : newPermissions) { @@ -264,6 +317,11 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation */ private SecurityGroup getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(String nodeId, String locationId, SecurityGroupExtension securityApi) { Set<SecurityGroup> groupsOnNode = securityApi.listSecurityGroupsForNode(nodeId); + + if(groupsOnNode == null || groupsOnNode.isEmpty()){ + return null; + } + SecurityGroup unique; if (locationId.equals("aws-ec2")) { if (groupsOnNode.size() == 2) { @@ -490,6 +548,17 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation return runOperationWithRetry(callable); } + protected SecurityGroup removePermission(final IpPermission permission, final SecurityGroup group, final SecurityGroupExtension securityApi) { + LOG.debug("Removing permission from security group {}: {}", group.getName(), permission); + Callable<SecurityGroup> callable = new Callable<SecurityGroup>() { + @Override + public SecurityGroup call() throws Exception { + return securityApi.removeIpPermission(permission, group); + } + }; + return runOperationWithRetry(callable); + } + /** @return the CIDR block used to configure Brooklyn's in security groups */ public String getBrooklynCidrBlock() { return sshCidrSupplier.get().toString(); http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/a4de7439/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 aafcf6e..e88ef21 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 @@ -29,6 +29,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; import static org.testng.Assert.assertNotEquals; +import static org.testng.Assert.assertNotNull; import static org.testng.Assert.assertTrue; import java.net.URI; @@ -53,6 +54,7 @@ import com.google.common.base.Predicate; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.util.concurrent.UncheckedExecutionException; import org.apache.brooklyn.location.jclouds.JcloudsLocation; import org.apache.brooklyn.util.collections.MutableMap; @@ -147,6 +149,59 @@ public class JcloudsLocationSecurityGroupCustomizerTest { } @Test + public void testRemovePermissionsFromNode() { + IpPermission ssh = newPermission(22); + IpPermission jmx = newPermission(31001); + String nodeId = "node"; + SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup()); + SecurityGroup group = newGroup("id"); + when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group)); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); + + customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService); + customizer.removePermissionsFromLocation(ImmutableList.of(jmx), nodeId, computeService); + + verify(securityApi, never()).removeIpPermission(ssh, group); + verify(securityApi, times(1)).removeIpPermission(jmx, group); + } + + @Test + public void testRemoveMultiplePermissionsFromNode() { + IpPermission ssh = newPermission(22); + IpPermission jmx = newPermission(31001); + String nodeId = "node"; + SecurityGroup sharedGroup = newGroup(customizer.getNameForSharedSecurityGroup()); + SecurityGroup group = newGroup("id"); + when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, group)); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); + + customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService); + customizer.removePermissionsFromLocation(ImmutableList.of(ssh, jmx), nodeId, computeService); + + verify(securityApi, times(1)).removeIpPermission(ssh, group); + verify(securityApi, times(1)).removeIpPermission(jmx, group); + } + + + @Test + public void testAddPermissionWhenNoExtension() { + IpPermission ssh = newPermission(22); + IpPermission jmx = newPermission(31001); + String nodeId = "node"; + + when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(Collections.<SecurityGroup>emptySet()); + + RuntimeException exception = null; + try { + customizer.addPermissionsToLocation(ImmutableList.of(ssh, jmx), nodeId, computeService); + } catch(RuntimeException e){ + exception = e; + } + + assertNotNull(exception); + } + + @Test public void testAddPermissionsToNodeUsesUncachedSecurityGroup() { JcloudsLocation jcloudsLocation = new JcloudsLocation(MutableMap.of("deferConstruction", true)); IpPermission ssh = newPermission(22);