Repository: incubator-brooklyn Updated Branches: refs/heads/master 664849507 -> ae4c9d31b
Fix BROOKLYN-99 fix JcloudsLocationSecurityGroupCustomizer for Nova's SecurityGroupExtension add more javadoc 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/57b81560 Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/57b81560 Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/57b81560 Branch: refs/heads/master Commit: 57b815603652a7297a7639a03231977f30b2a8ba Parents: 3650dfa Author: Andrea Turli <[email protected]> Authored: Tue Jun 16 22:33:41 2015 +0200 Committer: Andrea Turli <[email protected]> Committed: Tue Jun 23 14:20:48 2015 +0200 ---------------------------------------------------------------------- .../JcloudsLocationSecurityGroupCustomizer.java | 119 ++++++++++++++----- ...oudsLocationSecurityGroupCustomizerTest.java | 9 +- 2 files changed, 94 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/57b81560/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java b/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java index 7d28a9f..c3c4b27 100644 --- a/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java +++ b/locations/jclouds/src/main/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizer.java @@ -25,6 +25,8 @@ import java.util.Set; import java.util.concurrent.Callable; import java.util.concurrent.ExecutionException; +import javax.annotation.Nullable; + import org.jclouds.aws.AWSResponseException; import org.jclouds.compute.ComputeService; import org.jclouds.compute.domain.SecurityGroup; @@ -33,6 +35,8 @@ import org.jclouds.compute.extensions.SecurityGroupExtension; import org.jclouds.domain.Location; import org.jclouds.net.domain.IpPermission; import org.jclouds.net.domain.IpProtocol; +import org.jclouds.providers.ProviderMetadata; +import org.jclouds.providers.Providers; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -41,7 +45,6 @@ import brooklyn.location.geo.LocalhostExternalIpLoader; import brooklyn.location.jclouds.BasicJcloudsLocationCustomizer; import brooklyn.location.jclouds.JcloudsLocation; import brooklyn.location.jclouds.JcloudsMachineLocation; -import brooklyn.management.ManagementContext; import brooklyn.util.exceptions.Exceptions; import brooklyn.util.net.Cidr; import brooklyn.util.task.Tasks; @@ -49,6 +52,7 @@ import brooklyn.util.time.Duration; import com.google.common.annotations.Beta; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Function; import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.base.Predicates; @@ -59,6 +63,7 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; 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.ImmutableSet; import com.google.common.collect.Iterables; @@ -66,6 +71,13 @@ import com.google.common.collect.Iterables; /** * Configures custom security groups on Jclouds locations. * + * @see SecurityGroupExtension is an optional extension to jclouds compute service. It allows the manipulation of + * {@link SecurityGroup}s. + * + * This customizer can be injected into {@link JcloudsLocation#obtainOnce} using + * It will be executed after the provisiioning of the {@link JcloudsMachineLocation} to apply app-specific + * customization related to the security groups. + * * @since 0.7.0 */ @Beta @@ -161,7 +173,7 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation addPermissionsToLocation(location, securityGroupDefinition.getPermissions()); return this; } - + /** * Applies the given security group permissions to the given location. * <p> @@ -192,6 +204,7 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation return; } final SecurityGroupExtension securityApi = computeService.getSecurityGroupExtension().get(); + final String locationId = computeService.getContext().unwrap().getId(); // 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. @@ -202,7 +215,7 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation try { machineUniqueSecurityGroup = uniqueGroupCache.get(nodeId, new Callable<SecurityGroup>() { @Override public SecurityGroup call() throws Exception { - SecurityGroup sg = getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(nodeId, securityApi); + SecurityGroup sg = getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(nodeId, locationId, securityApi); if (sg == null) { throw new IllegalStateException("Failed to find machine-unique group on node: " + nodeId); } @@ -225,41 +238,47 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation * {@link #sharedGroupCache} if no mapping for the shared group's location previously * existed (e.g. Brooklyn was restarted and rebound to an existing application). * + * Notice that jclouds will attach 2 securityGroups to the node if the locationId is `aws-ec2` so it needs to + * look for the uniqueSecurityGroup rather than the shared securityGroup. + * * @param nodeId The id of the node in question + * @param locationId The id of the location in question * @param securityApi The API to use to list security groups * @return the security group unique to the given node, or null if one could not be determined. */ - private SecurityGroup getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown( - String nodeId, SecurityGroupExtension securityApi) { + private SecurityGroup getUniqueSecurityGroupForNodeCachingSharedGroupIfPreviouslyUnknown(String nodeId, String locationId, SecurityGroupExtension securityApi) { Set<SecurityGroup> groupsOnNode = securityApi.listSecurityGroupsForNode(nodeId); - if (groupsOnNode.size() != 2) { - LOG.warn("Expected to find two security groups on node {} in app {} (one shared, one unique). Found {}: {}", - new Object[]{nodeId, applicationId, groupsOnNode.size(), groupsOnNode}); - return null; - } - String expectedSharedName = getNameForSharedSecurityGroup(); - Iterator<SecurityGroup> it = groupsOnNode.iterator(); - SecurityGroup shared = it.next(); SecurityGroup unique; - if (shared.getName().endsWith(expectedSharedName)) { - unique = it.next(); - } else { - unique = shared; - shared = it.next(); - } - if (!shared.getName().endsWith(expectedSharedName)) { - LOG.warn("Couldn't determine which security group is shared between instances in app {}. Expected={}, found={}", - new Object[]{applicationId, expectedSharedName, groupsOnNode}); - return null; - } - // Shared entry might be missing if Brooklyn has rebound to an application - SecurityGroup old = sharedGroupCache.asMap().putIfAbsent(shared.getLocation(), shared); - LOG.info("Loaded unique security group for node {} (in {}): {}", - new Object[]{nodeId, applicationId, unique}); - if (old == null) { - LOG.info("Proactively set shared group for app {} to: {}", applicationId, shared); + if (locationId.equals("aws-ec2")) { + if (groupsOnNode.size() != 2) { + LOG.warn("Expected to find two security groups on node {} in app {} (one shared, one unique). Found {}: {}", + new Object[]{nodeId, applicationId, groupsOnNode.size(), groupsOnNode}); + return null; + } + String expectedSharedName = getNameForSharedSecurityGroup(); + Iterator<SecurityGroup> it = groupsOnNode.iterator(); + SecurityGroup shared = it.next(); + if (shared.getName().endsWith(expectedSharedName)) { + unique = it.next(); + } else { + unique = shared; + shared = it.next(); + } + if (!shared.getName().endsWith(expectedSharedName)) { + LOG.warn("Couldn't determine which security group is shared between instances in app {}. Expected={}, found={}", + new Object[]{applicationId, expectedSharedName, groupsOnNode}); + return null; + } + // Shared entry might be missing if Brooklyn has rebound to an application + SecurityGroup old = sharedGroupCache.asMap().putIfAbsent(shared.getLocation(), shared); + LOG.info("Loaded unique security group for node {} (in {}): {}", + new Object[]{nodeId, applicationId, unique}); + if (old == null) { + LOG.info("Proactively set shared group for app {} to: {}", applicationId, shared); + } + return unique; } - return unique; + return Iterables.getOnlyElement(groupsOnNode); } /** @@ -348,6 +367,11 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation * <li>Allow SSH access on port 22 from the world</li> * <li>Allow TCP, UDP and ICMP communication between machines in the same group</li> * </ul> + * + * It needs to consider locationId as port ranges and groupId are cloud provider-dependent e.g openstack nova + * wants from 1-65535 while aws-ec2 accepts from 0-65535. + * + * * @param groupName The name of the security group to create * @param location The location in which the security group will be created * @param securityApi The API to use to create the security group @@ -357,11 +381,19 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation private SecurityGroup createBaseSecurityGroupInLocation(String groupName, Location location, SecurityGroupExtension securityApi) { SecurityGroup group = addSecurityGroupInLocation(groupName, location, securityApi); + Set<String> openstackNovaIds = getJcloudsLocationIds("openstack-nova"); + + String groupId = group.getProviderId(); + int fromPort = 0; + if (location.getParent() != null && Iterables.contains(openstackNovaIds, location.getParent().getId())) { + groupId = group.getId(); + fromPort = 1; + } // Note: For groupName to work with GCE we also need to tag the machines with the same ID. // See sourceTags section at https://developers.google.com/compute/docs/networking#firewalls IpPermission.Builder allWithinGroup = IpPermission.builder() - .groupId(group.getProviderId()) - .fromPort(0) + .groupId(groupId) + .fromPort(fromPort) .toPort(65535); addPermission(allWithinGroup.ipProtocol(IpProtocol.TCP).build(), group, securityApi); addPermission(allWithinGroup.ipProtocol(IpProtocol.UDP).build(), group, securityApi); @@ -378,6 +410,27 @@ public class JcloudsLocationSecurityGroupCustomizer extends BasicJcloudsLocation return group; } + private Set<String> getJcloudsLocationIds(final String jcloudsApiId) { + Set<String> openstackNovaProviders = FluentIterable.from(Providers.all()) + .filter(new Predicate<ProviderMetadata>() { + @Override + public boolean apply(ProviderMetadata providerMetadata) { + return providerMetadata.getApiMetadata().getId().equals(jcloudsApiId); + } + }).transform(new Function<ProviderMetadata, String>() { + @Nullable + @Override + public String apply(ProviderMetadata input) { + return input.getId(); + } + }).toSet(); + + return new ImmutableSet.Builder<String>() + .addAll(openstackNovaProviders) + .add(jcloudsApiId) + .build(); + } + protected SecurityGroup addSecurityGroupInLocation(final String groupName, final Location location, final SecurityGroupExtension securityApi) { LOG.debug("Creating security group {} in {}", groupName, location); Callable<SecurityGroup> callable = new Callable<SecurityGroup>() { http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/57b81560/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java ---------------------------------------------------------------------- diff --git a/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java b/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java index 643f78f..69d962f 100644 --- a/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java +++ b/locations/jclouds/src/test/java/brooklyn/location/jclouds/networking/JcloudsLocationSecurityGroupCustomizerTest.java @@ -44,6 +44,7 @@ import org.jclouds.compute.options.TemplateOptions; import org.jclouds.domain.Location; import org.jclouds.net.domain.IpPermission; import org.jclouds.net.domain.IpProtocol; +import org.mockito.Answers; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -76,7 +77,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { customizer = new JcloudsLocationSecurityGroupCustomizer("testapp", new TestCidrSupplier()); location = mock(Location.class); securityApi = mock(SecurityGroupExtension.class); - computeService = mock(ComputeService.class); + computeService = mock(ComputeService.class, Answers.RETURNS_DEEP_STUBS.get()); when(computeService.getSecurityGroupExtension()).thenReturn(Optional.of(securityApi)); } @@ -136,6 +137,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { 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); @@ -157,6 +159,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { when(template.getLocation()).thenReturn(location); when(template.getOptions()).thenReturn(templateOptions); when(securityApi.createSecurityGroup(anyString(), eq(location))).thenReturn(sharedGroup); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); // Call customize to cache the shared group customizer.customize(jcloudsLocation, computeService, template); @@ -177,6 +180,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { SecurityGroup uniqueGroup = newGroup("unique"); when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup)); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); // Expect first call to list security groups on nodeId, second to use cached version customizer.addPermissionsToLocation(ImmutableSet.of(ssh), nodeId, computeService); @@ -196,6 +200,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { when(securityApi.listSecurityGroupsForNode(nodeId)).thenReturn(ImmutableSet.of(sharedGroup, uniqueGroup)); when(securityApi.addIpPermission(eq(ssh), eq(uniqueGroup))) .thenThrow(new RuntimeException("exception creating " + ssh)); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); try { customizer.addPermissionsToLocation(ImmutableList.of(ssh), nodeId, computeService); @@ -224,6 +229,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { } }; customizer.setRetryExceptionPredicate(messageChecker); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); IpPermission ssh = newPermission(22); String nodeId = "node"; @@ -255,6 +261,7 @@ public class JcloudsLocationSecurityGroupCustomizerTest { .thenThrow(newAwsResponseExceptionWithCode("RequestLimitExceeded")) .thenThrow(newAwsResponseExceptionWithCode("Blocked")) .thenReturn(sharedGroup); + when(computeService.getContext().unwrap().getId()).thenReturn("aws-ec2"); try { customizer.addPermissionsToLocation(ImmutableList.of(ssh), nodeId, computeService);
