Return-Path: X-Original-To: apmail-brooklyn-commits-archive@minotaur.apache.org Delivered-To: apmail-brooklyn-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 13ADA18032 for ; Tue, 15 Dec 2015 15:15:29 +0000 (UTC) Received: (qmail 92320 invoked by uid 500); 15 Dec 2015 15:15:29 -0000 Delivered-To: apmail-brooklyn-commits-archive@brooklyn.apache.org Received: (qmail 92270 invoked by uid 500); 15 Dec 2015 15:15:28 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 92250 invoked by uid 99); 15 Dec 2015 15:15:28 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 15 Dec 2015 15:15:28 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 9C490DFDEA; Tue, 15 Dec 2015 15:15:28 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: sjcorbett@apache.org To: commits@brooklyn.apache.org Date: Tue, 15 Dec 2015 15:15:28 -0000 Message-Id: <01915033978e4c85aff3066e7e0b5876@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] incubator-brooklyn git commit: Ported some Clocker changes back to brooklyn. It is now possible to remove security groups permissions Also added some error handling to JcloudsLocationSecurityGroupCustomizer 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 Authored: Thu Dec 10 15:37:01 2015 +0000 Committer: Graeme-Miller 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() { + @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. *

@@ -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. + *

+ * 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 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. + *

+ * 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 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() { - @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 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 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 callable = new Callable() { + @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.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);