JCLOUDS-287. Add SecurityGroupExtension support to CloudStack.

apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/config/CloudStackComputeServiceContextModule.java

@@ -30,8 +30,8 @@ import javax.inject.Named;
 import javax.inject.Singleton;
 
 import org.jclouds.cloudstack.CloudStackApi;
-import org.jclouds.cloudstack.compute.CloudStackComputeService;
 import org.jclouds.cloudstack.compute.extensions.CloudStackImageExtension;
+import org.jclouds.cloudstack.compute.extensions.CloudStackSecurityGroupExtension;
 import org.jclouds.cloudstack.compute.functions.CloudStackSecurityGroupToSecurityGroup;
 import org.jclouds.cloudstack.compute.functions.IngressRuleToIpPermission;
 import org.jclouds.cloudstack.compute.functions.OrphanedGroupsByZoneId;
@@ -48,8 +48,8 @@ import org.jclouds.cloudstack.compute.strategy.BasicNetworkOptionsConverter;
 import org.jclouds.cloudstack.compute.strategy.CloudStackComputeServiceAdapter;
 import org.jclouds.cloudstack.compute.strategy.OptionsConverter;
 import org.jclouds.cloudstack.domain.FirewallRule;
-import org.jclouds.cloudstack.domain.IngressRule;
 import org.jclouds.cloudstack.domain.IPForwardingRule;
+import org.jclouds.cloudstack.domain.IngressRule;
 import org.jclouds.cloudstack.domain.Network;
 import org.jclouds.cloudstack.domain.NetworkType;
 import org.jclouds.cloudstack.domain.OSType;
@@ -78,6 +78,7 @@ import org.jclouds.compute.config.ComputeServiceAdapterContextModule;
 import org.jclouds.compute.domain.NodeMetadata;
 import org.jclouds.compute.domain.OperatingSystem;
 import org.jclouds.compute.extensions.ImageExtension;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
 import org.jclouds.compute.options.TemplateOptions;
 import org.jclouds.domain.Location;
 import org.jclouds.net.domain.IpPermission;
@@ -99,8 +100,8 @@ import com.google.inject.Injector;
 import com.google.inject.Key;
 import com.google.inject.Provides;
 import com.google.inject.TypeLiteral;
-import com.google.inject.name.Names;
 import com.google.inject.assistedinject.FactoryModuleBuilder;
+import com.google.inject.name.Names;
 
 /**
  * 
@@ -150,8 +151,12 @@ public class CloudStackComputeServiceContextModule extends
       bind(new TypeLiteral<ImageExtension>() {
       }).to(CloudStackImageExtension.class);
 
+      bind(new TypeLiteral<SecurityGroupExtension>() {
+      }).to(CloudStackSecurityGroupExtension.class);
+
       // to have the compute service adapter override default locations
-      install(new LocationsFromComputeServiceAdapterModule<VirtualMachine, ServiceOffering, Template, Zone>(){});
+      install(new LocationsFromComputeServiceAdapterModule<VirtualMachine, ServiceOffering, Template, Zone>() {
+      });
    }
    
 
@@ -272,4 +277,8 @@ public class CloudStackComputeServiceContextModule extends
       return Optional.of(i.getInstance(ImageExtension.class));
    }
 
+   @Override
+   protected Optional<SecurityGroupExtension> provideSecurityGroupExtension(Injector i) {
+      return Optional.of(i.getInstance(SecurityGroupExtension.class));
+   }
 }

apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java

@@ -0,0 +1,283 @@
+/*
+ * 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.cloudstack.compute.extensions;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.base.Predicates.notNull;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.ruleCidrMatches;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.ruleGroupMatches;
+
+import java.util.Set;
+
+import javax.inject.Inject;
+
+import org.jclouds.cloudstack.CloudStackApi;
+import org.jclouds.cloudstack.domain.IngressRule;
+import org.jclouds.cloudstack.domain.ZoneAndName;
+import org.jclouds.cloudstack.domain.ZoneSecurityGroupNamePortsCidrs;
+import org.jclouds.cloudstack.options.ListSecurityGroupsOptions;
+import org.jclouds.cloudstack.strategy.BlockUntilJobCompletesAndReturnResult;
+import org.jclouds.collect.Memoized;
+import org.jclouds.compute.domain.SecurityGroup;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
+import org.jclouds.compute.functions.GroupNamingConvention;
+import org.jclouds.domain.Location;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+
+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.collect.ImmutableSet;
+import com.google.common.collect.Multimap;
+
+/**
+ * An extension to compute service to allow for the manipulation of {@link org.jclouds.compute.domain.SecurityGroup}s. Implementation
+ * is optional by providers.
+ *
+ * @author Andrew Bayer
+ */
+public class CloudStackSecurityGroupExtension implements SecurityGroupExtension {
+   protected final CloudStackApi api;
+   protected final Function<org.jclouds.cloudstack.domain.SecurityGroup,SecurityGroup> groupConverter;
+   protected final LoadingCache<ZoneAndName, org.jclouds.cloudstack.domain.SecurityGroup> groupCreator;
+   protected final GroupNamingConvention.Factory namingConvention;
+   protected final Supplier<Set<? extends Location>> locations;
+   protected final BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult;
+   protected final Predicate<String> jobComplete;
+
+   @Inject
+   public CloudStackSecurityGroupExtension(CloudStackApi api,
+                                           Function<org.jclouds.cloudstack.domain.SecurityGroup,SecurityGroup> groupConverter,
+                                           LoadingCache<ZoneAndName, org.jclouds.cloudstack.domain.SecurityGroup> groupCreator,
+                                           GroupNamingConvention.Factory namingConvention,
+                                           @Memoized Supplier<Set<? extends Location>> locations,
+                                           BlockUntilJobCompletesAndReturnResult blockUntilJobCompletesAndReturnResult,
+                                           Predicate<String> jobComplete) {
+      this.api = checkNotNull(api, "api");
+      this.groupConverter = checkNotNull(groupConverter, "groupConverter");
+      this.groupCreator = checkNotNull(groupCreator, "groupCreator");
+      this.namingConvention = checkNotNull(namingConvention, "namingConvention");
+      this.locations = checkNotNull(locations, "locations");
+      this.blockUntilJobCompletesAndReturnResult = checkNotNull(blockUntilJobCompletesAndReturnResult,
+              "blockUntilJobCompletesAndReturnResult");
+      this.jobComplete = checkNotNull(jobComplete, "jobComplete");
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroups() {
+      Iterable<? extends org.jclouds.cloudstack.domain.SecurityGroup> rawGroups =
+              api.getSecurityGroupApi().listSecurityGroups();
+      Iterable<SecurityGroup> groups = transform(filter(rawGroups, notNull()),
+              groupConverter);
+      return ImmutableSet.copyOf(groups);
+   }
+
+   /**
+    * Note that for the time being, security groups are not scoped by location in
+    * CloudStack, so this will simply return listSecurityGroups().
+    *
+    * @param location
+    * @return security groups
+    */
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsInLocation(final Location location) {
+      return listSecurityGroups();
+   }
+
+   @Override
+   public Set<SecurityGroup> listSecurityGroupsForNode(String id) {
+      checkNotNull(id, "id");
+
+      Iterable<? extends org.jclouds.cloudstack.domain.SecurityGroup> rawGroups =
+              api.getSecurityGroupApi().listSecurityGroups(ListSecurityGroupsOptions.Builder
+                      .virtualMachineId(id));
+
+      Iterable<SecurityGroup> groups = transform(filter(rawGroups, notNull()),
+              groupConverter);
+      return ImmutableSet.copyOf(groups);
+   }
+
+   @Override
+   public SecurityGroup getSecurityGroupById(String id) {
+      checkNotNull(id, "id");
+
+      org.jclouds.cloudstack.domain.SecurityGroup rawGroup
+              = api.getSecurityGroupApi().getSecurityGroup(id);
+
+      if (rawGroup == null) {
+         return null;
+      }
+
+      return groupConverter.apply(rawGroup);
+   }
+
+   @Override
+   public SecurityGroup createSecurityGroup(String name, Location location) {
+      checkNotNull(name, "name");
+      checkNotNull(location, "location");
+
+      String markerGroup = namingConvention.create().sharedNameForGroup(name);
+
+      ZoneSecurityGroupNamePortsCidrs zoneAndName = ZoneSecurityGroupNamePortsCidrs.builder()
+              .zone(location.getId())
+              .name(markerGroup)
+              .build();
+
+      return groupConverter.apply(groupCreator.apply(zoneAndName));
+   }
+
+   @Override
+   public boolean removeSecurityGroup(String id) {
+      checkNotNull(id, "id");
+
+      org.jclouds.cloudstack.domain.SecurityGroup group =
+              api.getSecurityGroupApi().getSecurityGroup(id);
+
+      if (group != null) {
+         for (IngressRule rule : group.getIngressRules()) {
+            jobComplete.apply(api.getSecurityGroupApi().revokeIngressRule(rule.getId()));
+         }
+
+         api.getSecurityGroupApi().deleteSecurityGroup(id);
+         // TODO find something better here maybe - hard to map zones to groups
+         for (Location location : locations.get()) {
+            groupCreator.invalidate(ZoneSecurityGroupNamePortsCidrs.builder()
+                    .zone(location.getId())
+                    .name(group.getName())
+                    .build());
+         }
+
+         return true;
+      }
+
+      return false;
+   }
+
+   @Override
+   public SecurityGroup addIpPermission(IpPermission ipPermission, SecurityGroup group) {
+      String id = checkNotNull(group.getId(), "group.getId()");
+
+      if (ipPermission.getCidrBlocks().size() > 0) {
+         jobComplete.apply(api.getSecurityGroupApi().authorizeIngressPortsToCIDRs(id,
+                 ipPermission.getIpProtocol().toString().toUpperCase(),
+                 ipPermission.getFromPort(),
+                 ipPermission.getToPort(),
+                 ipPermission.getCidrBlocks()));
+      }
+
+      if (ipPermission.getTenantIdGroupNamePairs().size() > 0) {
+         jobComplete.apply(api.getSecurityGroupApi().authorizeIngressPortsToSecurityGroups(id,
+                 ipPermission.getIpProtocol().toString().toUpperCase(),
+                 ipPermission.getFromPort(),
+                 ipPermission.getToPort(),
+                 ipPermission.getTenantIdGroupNamePairs()));
+      }
+
+      return getSecurityGroupById(id);
+   }
+
+   @Override
+   public SecurityGroup addIpPermission(IpProtocol protocol, int startPort, int endPort,
+                                        Multimap<String, String> tenantIdGroupNamePairs,
+                                        Iterable<String> ipRanges,
+                                        Iterable<String> groupIds, SecurityGroup group) {
+      IpPermission.Builder permBuilder = IpPermission.builder();
+      permBuilder.ipProtocol(protocol);
+      permBuilder.fromPort(startPort);
+      permBuilder.toPort(endPort);
+      permBuilder.tenantIdGroupNamePairs(tenantIdGroupNamePairs);
+      permBuilder.cidrBlocks(ipRanges);
+      permBuilder.groupIds(groupIds);
+
+      return addIpPermission(permBuilder.build(), group);
+   }
+
+   @Override
+   public SecurityGroup removeIpPermission(IpPermission ipPermission, SecurityGroup group) {
+      String id = checkNotNull(group.getId(), "group.getId()");
+
+      org.jclouds.cloudstack.domain.SecurityGroup rawGroup = api.getSecurityGroupApi()
+              .getSecurityGroup(id);
+
+      if (ipPermission.getCidrBlocks().size() > 0) {
+         for (IngressRule rule : filter(rawGroup.getIngressRules(),
+                 ruleCidrMatches(ipPermission.getIpProtocol().toString(),
+                         ipPermission.getFromPort(),
+                         ipPermission.getToPort(),
+                         ipPermission.getCidrBlocks()))) {
+             jobComplete.apply(api.getSecurityGroupApi().revokeIngressRule(rule.getId()));
+         }
+      }
+
+      if (ipPermission.getTenantIdGroupNamePairs().size() > 0) {
+         for (IngressRule rule : filter(rawGroup.getIngressRules(),
+                 ruleGroupMatches(ipPermission.getIpProtocol().toString(),
+                         ipPermission.getFromPort(),
+                         ipPermission.getToPort(),
+                         ipPermission.getTenantIdGroupNamePairs()))) {
+             jobComplete.apply(api.getSecurityGroupApi().revokeIngressRule(rule.getId()));
+         }
+      }
+
+      return getSecurityGroupById(id);
+   }
+
+   @Override
+   public SecurityGroup removeIpPermission(IpProtocol protocol, int startPort, int endPort,
+                                           Multimap<String, String> tenantIdGroupNamePairs,
+                                           Iterable<String> ipRanges,
+                                           Iterable<String> groupIds, SecurityGroup group) {
+      IpPermission.Builder permBuilder = IpPermission.builder();
+      permBuilder.ipProtocol(protocol);
+      permBuilder.fromPort(startPort);
+      permBuilder.toPort(endPort);
+      permBuilder.tenantIdGroupNamePairs(tenantIdGroupNamePairs);
+      permBuilder.cidrBlocks(ipRanges);
+      permBuilder.groupIds(groupIds);
+
+      return removeIpPermission(permBuilder.build(), group);
+   }
+
+   @Override
+   public boolean supportsTenantIdGroupNamePairs() {
+      return true;
+   }
+
+   @Override
+   public boolean supportsTenantIdGroupIdPairs() {
+      return false;
+   }
+
+   @Override
+   public boolean supportsGroupIds() {
+      return false;
+   }
+
+   @Override
+   public boolean supportsPortRangesForGroups() {
+      return false;
+   }
+
+   protected Iterable<? extends org.jclouds.cloudstack.domain.SecurityGroup> pollSecurityGroups() {
+      return api.getSecurityGroupApi().listSecurityGroups();
+   }
+
+}

apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/functions/IngressRuleToIpPermission.java

@@ -48,8 +48,13 @@ public class IngressRuleToIpPermission implements Function<IngressRule, IpPermis
       builder.ipProtocol(IpProtocol.fromValue(rule.getProtocol()));
       builder.fromPort(rule.getStartPort());
       builder.toPort(rule.getEndPort());
-      builder.cidrBlock(rule.getCIDR());
+      if (rule.getCIDR() != null) {
-      
+         builder.cidrBlock(rule.getCIDR());
+      }
+      if (rule.getSecurityGroupName() != null
+              && rule.getAccount() != null) {
+         builder.tenantIdGroupNamePair(rule.getAccount(), rule.getSecurityGroupName());
+      }
       return builder.build();
    }
 }

apis/cloudstack/src/main/java/org/jclouds/cloudstack/features/SecurityGroupApi.java

@@ -239,6 +239,7 @@ public interface SecurityGroupApi {
    @Named("deleteSecurityGroup")
    @GET
    @QueryParams(keys = "command", values = "deleteSecurityGroup")
+   @Consumes(MediaType.APPLICATION_JSON)
    @Fallback(VoidOnNotFoundOr404.class)
    void deleteSecurityGroup(@QueryParam("id") String id);
 

apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicates.java

@@ -19,11 +19,14 @@ package org.jclouds.cloudstack.predicates;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static com.google.common.base.Predicates.alwaysTrue;
 
+import java.util.Set;
+
 import org.jclouds.cloudstack.domain.IngressRule;
 import org.jclouds.cloudstack.domain.SecurityGroup;
 
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
+import com.google.common.collect.Multimap;
 
 /**
  * 
@@ -158,4 +161,80 @@ public class SecurityGroupPredicates {
          }
       };
    }
+
+   /**
+    * matches IngressRules with the given protocol, start and end port, and
+    * any of the given CIDRs.
+    *
+    * @param protocol
+    * @param startPort
+    * @param endPort
+    * @param cidrs
+    *
+    * @return predicate that matches as described
+    */
+   public static Predicate<IngressRule> ruleCidrMatches(final String protocol,
+                                                        final int startPort,
+                                                        final int endPort,
+                                                        final Set<String> cidrs) {
+      checkNotNull(protocol, "protocol");
+      checkNotNull(cidrs, "cidrs");
+
+      return new Predicate<IngressRule>() {
+         @Override
+         public boolean apply(IngressRule rule) {
+            return protocol.equals(rule.getProtocol())
+                    && startPort == rule.getStartPort()
+                    && endPort == rule.getEndPort()
+                    && cidrs.contains(rule.getCIDR());
+         }
+
+         @Override
+         public String toString() {
+            return "ruleCidrMatches(" + protocol
+                    + "," + startPort
+                    + "," + endPort
+                    + ",[" + cidrs
+                    + "])";
+         }
+      };
+   }
+
+   /**
+    * matches IngressRules with the given protocol, start and end port, and
+    * any of the given account/security group name pairs.
+    *
+    * @param protocol
+    * @param startPort
+    * @param endPort
+    * @param accountGroupNames
+    *
+    * @return predicate that matches as described
+    */
+   public static Predicate<IngressRule> ruleGroupMatches(final String protocol,
+                                                         final int startPort,
+                                                         final int endPort,
+                                                         final Multimap<String,String> accountGroupNames) {
+      checkNotNull(protocol, "protocol");
+      checkNotNull(accountGroupNames, "accountGroupNames");
+
+      return new Predicate<IngressRule>() {
+         @Override
+         public boolean apply(IngressRule rule) {
+            return protocol.equals(rule.getProtocol())
+                    && startPort == rule.getStartPort()
+                    && endPort == rule.getEndPort()
+                    && accountGroupNames.containsEntry(rule.getAccount(), rule.getSecurityGroupName());
+         }
+
+         @Override
+         public String toString() {
+            return "ruleGroupMatches(" + protocol
+                    + "," + startPort
+                    + "," + endPort
+                    + ",[" + accountGroupNames
+                    + "])";
+         }
+      };
+   }
 }

apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionExpectTest.java

@@ -0,0 +1,703 @@
+/*
+ * 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.cloudstack.compute.extensions;
+
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertTrue;
+
+import java.util.Map;
+import java.util.Properties;
+import java.util.Set;
+
+import org.jclouds.cloudstack.CloudStackContext;
+import org.jclouds.cloudstack.compute.functions.ZoneToLocationTest;
+import org.jclouds.cloudstack.internal.BaseCloudStackComputeServiceContextExpectTest;
+import org.jclouds.compute.ComputeService;
+import org.jclouds.compute.domain.SecurityGroup;
+import org.jclouds.compute.domain.SecurityGroupBuilder;
+import org.jclouds.compute.extensions.SecurityGroupExtension;
+import org.jclouds.http.HttpRequest;
+import org.jclouds.http.HttpResponse;
+import org.jclouds.net.domain.IpPermission;
+import org.jclouds.net.domain.IpProtocol;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Function;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.LinkedHashMultimap;
+import com.google.common.collect.Multimap;
+import com.google.common.collect.Sets;
+import com.google.common.util.concurrent.UncheckedExecutionException;
+import com.google.inject.Module;
+
+/**
+ * 
+ * @author Andrew Bayer
+ */
+@Test(groups = "unit", testName = "CloudStackSecurityGroupExtensionExpectTest")
+public class CloudStackSecurityGroupExtensionExpectTest extends BaseCloudStackComputeServiceContextExpectTest<ComputeService> {
+
+   protected final HttpResponse addRuleResponse = HttpResponse.builder().statusCode(200)
+           .payload(payloadFromResource("/authorizesecuritygroupingressresponse.json"))
+           .build();
+
+   protected final HttpResponse revokeRuleResponse = HttpResponse.builder().statusCode(200)
+           .payload(payloadFromResource("/revokesecuritygroupingressresponse.json"))
+           .build();
+
+   protected final HttpRequest queryAsyncJobResultAuthorizeIngress = HttpRequest.builder().method("GET")
+           .endpoint("http://localhost:8080/client/api")
+           .addQueryParam("response", "json")
+           .addQueryParam("command", "queryAsyncJobResult")
+           .addQueryParam("jobid", "13330fc9-8b3e-4582-aa3e-90883c041010")
+           .addQueryParam("apiKey", "APIKEY")
+           .addQueryParam("signature", "y4gk3ckWAMPDNZM26LUK0gAhfiE%3D")
+           .addHeader("Accept", "application/json")
+           .build();
+
+   protected final HttpResponse queryAsyncJobResultAuthorizeIngressResponse = HttpResponse.builder().statusCode(200)
+           .payload(payloadFromResource("/queryasyncjobresultresponse-authorizeingress.json"))
+           .build();
+
+   protected final HttpRequest getWithRule = HttpRequest.builder().method("GET")
+           .endpoint("http://localhost:8080/client/api")
+           .addQueryParam("response", "json")
+           .addQueryParam("command", "listSecurityGroups")
+           .addQueryParam("listAll", "true")
+           .addQueryParam("id", "13")
+           .addQueryParam("apiKey", "APIKEY")
+           .addQueryParam("signature", "TmlGaO2ICM%2BiXQr88%2BZCyWUniSw%3D")
+           .addHeader("Accept", "application/json")
+           .build();
+
+   protected final HttpResponse getEmptyResponse = HttpResponse.builder().statusCode(200)
+           .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_empty.json"))
+           .build();
+
+   @Override
+   protected Properties setupProperties() {
+      Properties overrides = super.setupProperties();
+      overrides.setProperty("jclouds.zones", "MTV-Zone1");
+      return overrides;
+   }
+
+   public void testListSecurityGroups() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "o%2Bd8xxWT1Pa%2BI57SG2caFAblBYA%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/listsecuritygroupsresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(getZone, getZoneResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      Set<SecurityGroup> groups = extension.listSecurityGroups();
+      assertEquals(groups.size(), 5);
+   }
+
+   public void testListSecurityGroupsForNode() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("virtualmachineid", "some-node")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "x4f9fGMjIHXl5biaaFK5oOEONcg%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/listsecuritygroupsresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(getZone, getZoneResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      Set<SecurityGroup> groups = extension.listSecurityGroupsForNode("some-node");
+      assertEquals(groups.size(), 5);
+   }
+
+   public void testGetSecurityGroupById() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("id", "13")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "TmlGaO2ICM%2BiXQr88%2BZCyWUniSw%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(getZone, getZoneResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      SecurityGroup group = extension.getSecurityGroupById("13");
+      assertEquals(group.getId(), "13");
+      assertEquals(group.getIpPermissions().size(), 2);
+   }
+
+   public void testCreateSecurityGroup() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("securitygroupname", "jclouds-test")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "zGp2rfHY6fBIGkgODRxyNzFfPFI%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(getZoneWithSecurityGroups, getZoneWithSecurityGroupsResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .put(createSecurityGroup, createSecurityGroupResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      SecurityGroup group = extension.createSecurityGroup("test", ZoneToLocationTest.two);
+      assertEquals(group.getId(), "30");
+      assertEquals(group.getIpPermissions().size(), 0);
+   }
+
+   @Test(expectedExceptions = UncheckedExecutionException.class,
+           expectedExceptionsMessageRegExp = "java.lang.IllegalArgumentException: .* does not support security groups")
+   public void testCreateSecurityGroupBadZone() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("securitygroupname", "jclouds-test")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "zGp2rfHY6fBIGkgODRxyNzFfPFI%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(getZone, getZoneResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .put(createSecurityGroup, createSecurityGroupResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      SecurityGroup group = extension.createSecurityGroup("test", ZoneToLocationTest.one);
+      assertEquals(group.getId(), "30");
+      assertEquals(group.getIpPermissions().size(), 0);
+   }
+
+   public void testRemoveSecurityGroup() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("id", "13")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "TmlGaO2ICM%2BiXQr88%2BZCyWUniSw%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_empty.json"))
+              .build();
+
+      HttpRequest deleteSecurityGroup = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "deleteSecurityGroup")
+              .addQueryParam("id", "13")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "S1A2lYR/ibf4%2BHGFxVLdZvXZujQ%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse deleteSecurityGroupResponse = HttpResponse.builder()
+              .statusCode(200)
+              .payload(payloadFromResource("/deletesecuritygroupresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .put(deleteSecurityGroup, deleteSecurityGroupResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      assertTrue(extension.removeSecurityGroup("13"), "Did not remove security group");
+   }
+
+   public void testRemoveSecurityGroupDoesNotExist() {
+      HttpRequest listSecurityGroups = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "listSecurityGroups")
+              .addQueryParam("listAll", "true")
+              .addQueryParam("id", "14")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "pWQ30A6l5qh4eaNypGwM9FoLnUM%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse listSecurityGroupsResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse.json"))
+              .build();
+
+      Map<HttpRequest, HttpResponse> requestResponseMap = ImmutableMap.<HttpRequest, HttpResponse> builder()
+              .put(listTemplates, listTemplatesResponse)
+              .put(listOsTypes, listOsTypesResponse)
+              .put(listOsCategories, listOsCategoriesResponse)
+              .put(listZones, listZonesResponse)
+              .put(listServiceOfferings, listServiceOfferingsResponse)
+              .put(listAccounts, listAccountsResponse)
+              .put(listNetworks, listNetworksResponse)
+              .put(listSecurityGroups, listSecurityGroupsResponse)
+              .build();
+
+      SecurityGroupExtension extension = requestsSendResponses(requestResponseMap).getSecurityGroupExtension().get();
+
+      assertFalse(extension.removeSecurityGroup("14"), "Should not have found security group to remove");
+   }
+
+   public void testAddIpPermissionCidrFromIpPermission() {
+      HttpRequest addRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "authorizeSecurityGroupIngress")
+              .addQueryParam("securitygroupid", "13")
+              .addQueryParam("protocol", "UDP")
+              .addQueryParam("startport", "11")
+              .addQueryParam("endport", "11")
+              .addQueryParam("cidrlist", "1.1.1.1/24")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "XyokGNutHwcyU7KQVFZOTHvc4RY%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_cidr.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(addRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(addRuleResponse, queryAsyncJobResultAuthorizeIngressResponse, getWithRuleResponse)
+      ).getSecurityGroupExtension().get();
+
+      IpPermission.Builder builder = IpPermission.builder();
+
+      builder.ipProtocol(IpProtocol.UDP);
+      builder.fromPort(11);
+      builder.toPort(11);
+      builder.cidrBlock("1.1.1.1/24");
+
+      IpPermission perm = builder.build();
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.addIpPermission(perm, origGroup);
+
+      assertEquals(1, newGroup.getIpPermissions().size());
+
+      IpPermission newPerm = Iterables.getOnlyElement(newGroup.getIpPermissions());
+
+      assertNotNull(newPerm);
+      assertEquals(newPerm.getIpProtocol(), IpProtocol.UDP);
+      assertEquals(newPerm.getFromPort(), 11);
+      assertEquals(newPerm.getToPort(), 11);
+      assertEquals(newPerm.getCidrBlocks().size(), 1);
+      assertTrue(newPerm.getCidrBlocks().contains("1.1.1.1/24"));
+   }
+
+   public void testAddIpPermissionCidrFromParams() {
+      HttpRequest addRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "authorizeSecurityGroupIngress")
+              .addQueryParam("securitygroupid", "13")
+              .addQueryParam("protocol", "UDP")
+              .addQueryParam("startport", "11")
+              .addQueryParam("endport", "11")
+              .addQueryParam("cidrlist", "1.1.1.1/24")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "XyokGNutHwcyU7KQVFZOTHvc4RY%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_cidr.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(addRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(addRuleResponse, queryAsyncJobResultAuthorizeIngressResponse, getWithRuleResponse)
+      ).getSecurityGroupExtension().get();
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.addIpPermission(IpProtocol.UDP, 11, 11, emptyMultimap(),
+              ImmutableSet.of("1.1.1.1/24"), emptyStringSet(), origGroup);
+
+      assertEquals(1, newGroup.getIpPermissions().size());
+
+      IpPermission newPerm = Iterables.getOnlyElement(newGroup.getIpPermissions());
+
+      assertNotNull(newPerm);
+      assertEquals(newPerm.getIpProtocol(), IpProtocol.UDP);
+      assertEquals(newPerm.getFromPort(), 11);
+      assertEquals(newPerm.getToPort(), 11);
+      assertEquals(newPerm.getCidrBlocks().size(), 1);
+      assertTrue(newPerm.getCidrBlocks().contains("1.1.1.1/24"));
+   }
+
+   public void testAddIpPermissionGroupFromIpPermission() {
+      HttpRequest addRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "authorizeSecurityGroupIngress")
+              .addQueryParam("securitygroupid", "13")
+              .addQueryParam("protocol", "TCP")
+              .addQueryParam("startport", "22")
+              .addQueryParam("endport", "22")
+              .addQueryParam("usersecuritygrouplist[0].account", "adrian")
+              .addQueryParam("usersecuritygrouplist[0].group", "adriancole")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "v2OgKc2IftwX9pfKq2Pw/Z2xh9w%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_group.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(addRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(addRuleResponse, queryAsyncJobResultAuthorizeIngressResponse, getWithRuleResponse)
+      ).getSecurityGroupExtension().get();
+
+      IpPermission.Builder builder = IpPermission.builder();
+
+      builder.ipProtocol(IpProtocol.TCP);
+      builder.fromPort(22);
+      builder.toPort(22);
+      builder.tenantIdGroupNamePair("adrian", "adriancole");
+
+      IpPermission perm = builder.build();
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.addIpPermission(perm, origGroup);
+
+      assertEquals(1, newGroup.getIpPermissions().size());
+
+      IpPermission newPerm = Iterables.getOnlyElement(newGroup.getIpPermissions());
+
+      assertNotNull(newPerm);
+      assertEquals(newPerm.getIpProtocol(), IpProtocol.TCP);
+      assertEquals(newPerm.getFromPort(), 22);
+      assertEquals(newPerm.getToPort(), 22);
+      assertEquals(newPerm.getCidrBlocks().size(), 0);
+      assertEquals(newPerm.getTenantIdGroupNamePairs().size(), 1);
+      assertTrue(newPerm.getTenantIdGroupNamePairs().containsEntry("adrian", "adriancole"));
+   }
+
+   public void testAddIpPermissionGroupFromParams() {
+      HttpRequest addRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "authorizeSecurityGroupIngress")
+              .addQueryParam("securitygroupid", "13")
+              .addQueryParam("protocol", "TCP")
+              .addQueryParam("startport", "22")
+              .addQueryParam("endport", "22")
+              .addQueryParam("usersecuritygrouplist[0].account", "adrian")
+              .addQueryParam("usersecuritygrouplist[0].group", "adriancole")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "v2OgKc2IftwX9pfKq2Pw/Z2xh9w%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_group.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(addRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(addRuleResponse, queryAsyncJobResultAuthorizeIngressResponse, getWithRuleResponse)
+      ).getSecurityGroupExtension().get();
+
+      ImmutableMultimap.Builder<String, String> permBuilder = ImmutableMultimap.builder();
+      permBuilder.put("adrian", "adriancole");
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.addIpPermission(IpProtocol.TCP, 22, 22,
+              permBuilder.build(), emptyStringSet(), emptyStringSet(), origGroup);
+
+      assertEquals(1, newGroup.getIpPermissions().size());
+
+      IpPermission newPerm = Iterables.getOnlyElement(newGroup.getIpPermissions());
+
+      assertNotNull(newPerm);
+      assertEquals(newPerm.getIpProtocol(), IpProtocol.TCP);
+      assertEquals(newPerm.getFromPort(), 22);
+      assertEquals(newPerm.getToPort(), 22);
+      assertEquals(newPerm.getCidrBlocks().size(), 0);
+      assertEquals(newPerm.getTenantIdGroupNamePairs().size(), 1);
+      assertTrue(newPerm.getTenantIdGroupNamePairs().containsEntry("adrian", "adriancole"));
+   }
+
+   public void testRemoveIpPermissionCidrFromIpPermission() {
+      HttpRequest revokeRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "revokeSecurityGroupIngress")
+              .addQueryParam("id", "6")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "H7cY/MEYGN7df1hiz0mMAFVBfa8%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_cidr.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(getWithRule, revokeRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(getWithRuleResponse, revokeRuleResponse,
+                      queryAsyncJobResultAuthorizeIngressResponse, getEmptyResponse)
+      ).getSecurityGroupExtension().get();
+
+      IpPermission.Builder builder = IpPermission.builder();
+
+      builder.ipProtocol(IpProtocol.UDP);
+      builder.fromPort(11);
+      builder.toPort(11);
+      builder.cidrBlock("1.1.1.1/24");
+
+      IpPermission perm = builder.build();
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.removeIpPermission(perm, origGroup);
+
+      assertEquals(newGroup.getIpPermissions().size(), 0);
+   }
+
+   public void testRemoveIpPermissionCidrFromParams() {
+      HttpRequest revokeRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "revokeSecurityGroupIngress")
+              .addQueryParam("id", "6")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "H7cY/MEYGN7df1hiz0mMAFVBfa8%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_cidr.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(getWithRule, revokeRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(getWithRuleResponse, revokeRuleResponse,
+                      queryAsyncJobResultAuthorizeIngressResponse, getEmptyResponse)
+      ).getSecurityGroupExtension().get();
+
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.removeIpPermission(IpProtocol.UDP, 11, 11, emptyMultimap(),
+              ImmutableSet.of("1.1.1.1/24"), emptyStringSet(), origGroup);
+
+      assertEquals(newGroup.getIpPermissions().size(), 0);
+   }
+
+   public void testRemoveIpPermissionGroupFromIpPermission() {
+      HttpRequest revokeRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "revokeSecurityGroupIngress")
+              .addQueryParam("id", "5")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "bEzvrLtO7aEWkIqJgUeTnd%2B0XbY%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_group.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(getWithRule, revokeRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(getWithRuleResponse, revokeRuleResponse,
+                      queryAsyncJobResultAuthorizeIngressResponse, getEmptyResponse)
+      ).getSecurityGroupExtension().get();
+
+      IpPermission.Builder builder = IpPermission.builder();
+
+      builder.ipProtocol(IpProtocol.TCP);
+      builder.fromPort(22);
+      builder.toPort(22);
+      builder.tenantIdGroupNamePair("adrian", "adriancole");
+
+      IpPermission perm = builder.build();
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.removeIpPermission(perm, origGroup);
+
+      assertEquals(newGroup.getIpPermissions().size(), 0);
+   }
+
+   public void testRemoveIpPermissionGroupFromParams() {
+      HttpRequest revokeRule = HttpRequest.builder().method("GET")
+              .endpoint("http://localhost:8080/client/api")
+              .addQueryParam("response", "json")
+              .addQueryParam("command", "revokeSecurityGroupIngress")
+              .addQueryParam("id", "5")
+              .addQueryParam("apiKey", "APIKEY")
+              .addQueryParam("signature", "bEzvrLtO7aEWkIqJgUeTnd%2B0XbY%3D")
+              .addHeader("Accept", "application/json")
+              .build();
+
+      HttpResponse getWithRuleResponse = HttpResponse.builder().statusCode(200)
+              .payload(payloadFromResource("/getsecuritygroupresponse_extension_byid_with_group.json"))
+              .build();
+
+      SecurityGroupExtension extension = orderedRequestsSendResponses(
+              ImmutableList.of(getWithRule, revokeRule, queryAsyncJobResultAuthorizeIngress, getWithRule),
+              ImmutableList.of(getWithRuleResponse, revokeRuleResponse,
+                      queryAsyncJobResultAuthorizeIngressResponse, getEmptyResponse)
+      ).getSecurityGroupExtension().get();
+
+      ImmutableMultimap.Builder<String, String> permBuilder = ImmutableMultimap.builder();
+      permBuilder.put("adrian", "adriancole");
+
+      SecurityGroup origGroup = new SecurityGroupBuilder().id("13").build();
+
+      SecurityGroup newGroup = extension.removeIpPermission(IpProtocol.TCP, 22, 22,
+              permBuilder.build(), emptyStringSet(), emptyStringSet(), origGroup);
+
+      assertEquals(newGroup.getIpPermissions().size(), 0);
+   }
+
+   @Override
+   public ComputeService createClient(Function<HttpRequest, HttpResponse> fn, Module module, Properties props) {
+      return clientFrom(createInjector(fn, module, props).getInstance(CloudStackContext.class));
+   }
+
+   @Override
+   protected ComputeService clientFrom(CloudStackContext context) {
+      return context.getComputeService();
+   }
+
+   private Multimap<String, String> emptyMultimap() {
+      return LinkedHashMultimap.create();
+   }
+
+   private Set<String> emptyStringSet() {
+      return Sets.newLinkedHashSet();
+   }
+
+}

apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionLiveTest.java

@@ -0,0 +1,71 @@
+/*
+ * 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.cloudstack.compute.extensions;
+
+import org.jclouds.cloudstack.CloudStackApi;
+import org.jclouds.cloudstack.domain.Zone;
+import org.jclouds.compute.domain.Template;
+import org.jclouds.compute.extensions.internal.BaseSecurityGroupExtensionLiveTest;
+import org.jclouds.sshj.config.SshjSshClientModule;
+import org.testng.annotations.BeforeClass;
+import org.testng.annotations.Test;
+
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
+import com.google.inject.Module;
+
+/**
+ * Live test for CloudStack {@link org.jclouds.compute.extensions.SecurityGroupExtension} implementation.
+ *
+ * @author Andrew Bayer
+ *
+ */
+@Test(groups = "live", singleThreaded = true, testName = "CloudStackSecurityGroupExtensionLiveTest")
+public class CloudStackSecurityGroupExtensionLiveTest extends BaseSecurityGroupExtensionLiveTest {
+
+   protected Zone zone;
+
+   public CloudStackSecurityGroupExtensionLiveTest() {
+      provider = "cloudstack";
+   }
+
+   @BeforeClass(groups = { "integration", "live" })
+   public void setupContext() {
+      super.setupContext();
+
+      CloudStackApi api = view.unwrapApi(CloudStackApi.class);
+      zone = Iterables.find(api.getZoneApi().listZones(), new Predicate<Zone>() {
+
+         @Override
+         public boolean apply(Zone arg0) {
+            return arg0.isSecurityGroupsEnabled();
+         }
+
+      });
+      if (zone == null)
+         securityGroupsSupported = false;
+   }
+
+   protected Module getSshModule() {
+      return new SshjSshClientModule();
+   }
+
+   @Override
+   public Template getNodeTemplate() {
+      return view.getComputeService().templateBuilder().locationId(zone.getId()).build();
+   }
+}

apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/IngressRuleToIpPermissionTest.java

@@ -55,7 +55,7 @@ public class IngressRuleToIpPermissionTest {
       assertEquals(convertedPerm.getFromPort(), ruleToConvert.getStartPort());
       assertEquals(convertedPerm.getToPort(), ruleToConvert.getEndPort());
       assertEquals(convertedPerm.getCidrBlocks(), ImmutableSet.of("0.0.0.0/0"));
-      assertTrue(convertedPerm.getTenantIdGroupNamePairs().size() == 0);
+      assertTrue(convertedPerm.getTenantIdGroupNamePairs().size() == 1);
       assertTrue(convertedPerm.getGroupIds().size() == 0);
    }
 }

apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/functions/ZoneToLocationTest.java

@@ -41,12 +41,12 @@ import com.google.common.collect.Iterables;
 @Test(singleThreaded = true, groups = "unit")
 public class ZoneToLocationTest {
 
-   static JustProvider justProvider = new JustProvider("cloudstack", Suppliers.ofInstance(URI.create("foo")),
+   public static final JustProvider justProvider = new JustProvider("cloudstack", Suppliers.ofInstance(URI.create("foo")),
             ImmutableSet.<String> of());
-   static ZoneToLocation function = new ZoneToLocation(justProvider);
+   public static final ZoneToLocation function = new ZoneToLocation(justProvider);
-   static Location one = new LocationBuilder().parent(Iterables.get(justProvider.get(), 0)).scope(LocationScope.ZONE)
+   public static final Location one = new LocationBuilder().parent(Iterables.get(justProvider.get(), 0)).scope(LocationScope.ZONE)
          .description("San Jose 1").id("1").build();
-   static Location two = new LocationBuilder().parent(Iterables.get(justProvider.get(), 0)).scope(LocationScope.ZONE)
+   public static final Location two = new LocationBuilder().parent(Iterables.get(justProvider.get(), 0)).scope(LocationScope.ZONE)
          .description("Chicago").id("2").build();
 
    @Test

apis/cloudstack/src/test/java/org/jclouds/cloudstack/features/SecurityGroupApiTest.java

@@ -286,7 +286,7 @@ public class SecurityGroupApiTest extends BaseCloudStackApiTest<SecurityGroupApi
 
       assertRequestLineEquals(httpRequest,
             "GET http://localhost:8080/client/api?response=json&command=deleteSecurityGroup&id=5 HTTP/1.1");
-      assertNonPayloadHeadersEqual(httpRequest, "");
+      assertNonPayloadHeadersEqual(httpRequest, "Accept: application/json\n");
       assertPayloadEquals(httpRequest, null, null, false);
 
       assertResponseParserClassEquals(method, httpRequest, ReleasePayloadAndReturn.class);

apis/cloudstack/src/test/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicatesTest.java

@@ -16,22 +16,22 @@
  */
 package org.jclouds.cloudstack.predicates;
 
-import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.portInRange;
 import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.hasCidr;
-import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.portInRangeForCidr;
 import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.nameEquals;
-import static org.testng.Assert.assertEquals;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.portInRange;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.portInRangeForCidr;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.ruleCidrMatches;
+import static org.jclouds.cloudstack.predicates.SecurityGroupPredicates.ruleGroupMatches;
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 
-import java.util.Set;
-
 import org.jclouds.cloudstack.domain.IngressRule;
 import org.jclouds.cloudstack.domain.SecurityGroup;
-import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import com.google.common.collect.ImmutableMultimap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
 
 /**
  * @author Andrew Bayer
@@ -86,5 +86,25 @@ public class SecurityGroupPredicatesTest {
       assertTrue(nameEquals("default").apply(group()));
       assertFalse(nameEquals("not-default").apply(group()));
    }
-   
+
+   @Test
+   public void testRuleCidrMatches() {
+      assertTrue(Iterables.any(group().getIngressRules(),
+              ruleCidrMatches("tcp", 40, 50, ImmutableSet.of("1.1.1.1/24"))));
+      assertFalse(Iterables.any(group().getIngressRules(),
+              ruleCidrMatches("tcp", 40, 50, ImmutableSet.of("2.2.2.2/24"))));
+   }
+
+   @Test
+   public void testRuleGroupMatches() {
+      assertTrue(Iterables.any(group().getIngressRules(),
+              ruleGroupMatches("tcp", 22, 22,
+                      ImmutableMultimap.<String,String>builder().put("adrian", "adriancole").build())));
+      assertFalse(Iterables.any(group().getIngressRules(),
+              ruleGroupMatches("tcp", 22, 22,
+                      ImmutableMultimap.<String,String>builder().put("adrian", "somegroup").build())));
+      assertFalse(Iterables.any(group().getIngressRules(),
+              ruleGroupMatches("tcp", 22, 22,
+                      ImmutableMultimap.<String,String>builder().put("someuser", "adriancole").build())));
+   }
 }

apis/cloudstack/src/test/resources/deletesecuritygroupresponse.json

@@ -0,0 +1 @@
+{ "deletesecuritygroupresponse" : { "success" : "true"}  }

apis/cloudstack/src/test/resources/getsecuritygroupresponse_extension_byid.json

@@ -0,0 +1 @@
+{ "listsecuritygroupsresponse" : { "securitygroup" : [  {"id":13,"name":"default","description":"description","account":"adrian","domainid":1,"domain":"ROOT","ingressrule":[{"ruleid":5,"protocol":"tcp","startport":22,"endport":22,"securitygroupname":"adriancole","account":"adrian"},{"ruleid":6,"protocol":"udp","startport":11,"endport":11,"cidr":"1.1.1.1/24"}]} ] } }

apis/cloudstack/src/test/resources/getsecuritygroupresponse_extension_byid_empty.json

@@ -0,0 +1 @@
+{ "listsecuritygroupsresponse" : { "securitygroup" : [  {"id":13,"name":"default","description":"description","account":"adrian","domainid":1,"domain":"ROOT"} ] } }

apis/cloudstack/src/test/resources/getsecuritygroupresponse_extension_byid_with_cidr.json

@@ -0,0 +1 @@
+{ "listsecuritygroupsresponse" : { "securitygroup" : [  {"id":13,"name":"default","description":"description","account":"adrian","domainid":1,"domain":"ROOT","ingressrule":[{"ruleid":6,"protocol":"udp","startport":11,"endport":11,"cidr":"1.1.1.1/24"}]} ] } }

apis/cloudstack/src/test/resources/getsecuritygroupresponse_extension_byid_with_group.json

@@ -0,0 +1 @@
+{ "listsecuritygroupsresponse" : { "securitygroup" : [  {"id":13,"name":"default","description":"description","account":"adrian","domainid":1,"domain":"ROOT","ingressrule":[{"ruleid":5,"protocol":"tcp","startport":22,"endport":22,"securitygroupname":"adriancole","account":"adrian"}] }] }}

apis/cloudstack/src/test/resources/listzonesresponse_single.json

@@ -0,0 +1 @@
+{ "listzonesresponse" : { "zone" : [  {"id":2,"name":"Chicago","networktype":"Advanced","securitygroupsenabled":true} ] } }

apis/cloudstack/src/test/resources/revokesecuritygroupingressresponse.json

@@ -0,0 +1,2 @@
+ { "revokesecuritygroupingressresponse" :
+ {"jobid":"13330fc9-8b3e-4582-aa3e-90883c041010"} }

compute/src/test/java/org/jclouds/compute/extensions/internal/BaseSecurityGroupExtensionLiveTest.java

@@ -36,6 +36,7 @@ import org.jclouds.domain.Location;
 import org.jclouds.logging.Logger;
 import org.jclouds.net.domain.IpPermission;
 import org.jclouds.net.domain.IpProtocol;
+import org.testng.SkipException;
 import org.testng.annotations.AfterClass;
 import org.testng.annotations.Test;
 
@@ -65,6 +66,8 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
    protected String groupId;
 
+   protected boolean securityGroupsSupported = true;
+
    /**
     * Returns the template for the base node, override to test different templates.
     *
@@ -74,8 +77,15 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
       return view.getComputeService().templateBuilder().build();
    }
 
+   protected void skipIfSecurityGroupsNotSupported() {
+      if (!securityGroupsSupported) {
+         throw new SkipException("Test cannot run without security groups available.");
+      }
+   }
+
    @Test(groups = { "integration", "live" }, singleThreaded = true)
    public void testCreateSecurityGroup() throws RunNodesException, InterruptedException, ExecutionException {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -96,6 +106,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testCreateSecurityGroup")
    public void testGetSecurityGroupById() throws RunNodesException, InterruptedException, ExecutionException {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -114,6 +125,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testGetSecurityGroupById")
    public void testAddIpPermission() {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -143,6 +155,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
    
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testAddIpPermission")
    public void testRemoveIpPermission() {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -172,6 +185,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testRemoveIpPermission")
    public void testAddIpPermissionsFromSpec() {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -274,6 +288,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
    /*
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testAddIpPermissionsFromSpec")
    public void testCreateNodeWithSecurityGroup() throws RunNodesException, InterruptedException, ExecutionException {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -310,6 +325,7 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
    // instance is still floating around in EC2. - abayer, 6/14/13
    @Test(groups = { "integration", "live" }, singleThreaded = true, dependsOnMethods = "testAddIpPermissionsFromSpec")
    public void testDeleteSecurityGroup() {
+      skipIfSecurityGroupsNotSupported();
 
       ComputeService computeService = view.getComputeService();
 
@@ -343,17 +359,17 @@ public abstract class BaseSecurityGroupExtensionLiveTest extends BaseComputeServ
 
 
    private void cleanup() {
-      ComputeService computeService = view.getComputeService();
+      if (securityGroupsSupported) {
+         ComputeService computeService = view.getComputeService();
 
-      Location location = getNodeTemplate().getLocation();
+         Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension();
 
-      Optional<SecurityGroupExtension> securityGroupExtension = computeService.getSecurityGroupExtension();
+         if (securityGroupExtension.isPresent()) {
+            Optional<SecurityGroup> group = getGroup(securityGroupExtension.get());
 
-      if (securityGroupExtension.isPresent()) {
+            if (group.isPresent()) {
-         Optional<SecurityGroup> group = getGroup(securityGroupExtension.get());
+               securityGroupExtension.get().removeSecurityGroup(group.get().getId());
-
+            }
-         if (group.isPresent()) {
-            securityGroupExtension.get().removeSecurityGroup(group.get().getId());
          }
       }
    }