diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java index 599c37c6d0..0c7d54eff5 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtension.java @@ -151,31 +151,34 @@ public class CloudStackSecurityGroupExtension implements SecurityGroupExtension 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; + if (group == null) { + return false; } - return false; + 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; } @Override public SecurityGroup addIpPermission(IpPermission ipPermission, SecurityGroup group) { + checkNotNull(group, "group"); + checkNotNull(ipPermission, "ipPermission"); String id = checkNotNull(group.getId(), "group.getId()"); - if (ipPermission.getCidrBlocks().size() > 0) { + if (!ipPermission.getCidrBlocks().isEmpty()) { jobComplete.apply(api.getSecurityGroupApi().authorizeIngressPortsToCIDRs(id, ipPermission.getIpProtocol().toString().toUpperCase(), ipPermission.getFromPort(), @@ -183,7 +186,7 @@ public class CloudStackSecurityGroupExtension implements SecurityGroupExtension ipPermission.getCidrBlocks())); } - if (ipPermission.getTenantIdGroupNamePairs().size() > 0) { + if (!ipPermission.getTenantIdGroupNamePairs().isEmpty()) { jobComplete.apply(api.getSecurityGroupApi().authorizeIngressPortsToSecurityGroups(id, ipPermission.getIpProtocol().toString().toUpperCase(), ipPermission.getFromPort(), @@ -212,12 +215,14 @@ public class CloudStackSecurityGroupExtension implements SecurityGroupExtension @Override public SecurityGroup removeIpPermission(IpPermission ipPermission, SecurityGroup group) { + checkNotNull(group, "group"); + checkNotNull(ipPermission, "ipPermission"); String id = checkNotNull(group.getId(), "group.getId()"); org.jclouds.cloudstack.domain.SecurityGroup rawGroup = api.getSecurityGroupApi() .getSecurityGroup(id); - if (ipPermission.getCidrBlocks().size() > 0) { + if (!ipPermission.getCidrBlocks().isEmpty()) { for (IngressRule rule : filter(rawGroup.getIngressRules(), ruleCidrMatches(ipPermission.getIpProtocol().toString(), ipPermission.getFromPort(), @@ -227,7 +232,7 @@ public class CloudStackSecurityGroupExtension implements SecurityGroupExtension } } - if (ipPermission.getTenantIdGroupNamePairs().size() > 0) { + if (!ipPermission.getTenantIdGroupNamePairs().isEmpty()) { for (IngressRule rule : filter(rawGroup.getIngressRules(), ruleGroupMatches(ipPermission.getIpProtocol().toString(), ipPermission.getFromPort(), @@ -276,8 +281,4 @@ public class CloudStackSecurityGroupExtension implements SecurityGroupExtension return false; } - protected Iterable pollSecurityGroups() { - return api.getSecurityGroupApi().listSecurityGroups(); - } - } diff --git a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicates.java b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicates.java index 99b6e9e7dd..598b491138 100644 --- a/apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicates.java +++ b/apis/cloudstack/src/main/java/org/jclouds/cloudstack/predicates/SecurityGroupPredicates.java @@ -191,10 +191,10 @@ public class SecurityGroupPredicates { @Override public String toString() { - return "ruleCidrMatches(" + protocol - + "," + startPort - + "," + endPort - + ",[" + cidrs + return "ruleCidrMatches(protocol:" + protocol + + ",startPort:" + startPort + + ",endPort:" + endPort + + ",cidrs:[" + cidrs + "])"; } }; @@ -229,10 +229,10 @@ public class SecurityGroupPredicates { @Override public String toString() { - return "ruleGroupMatches(" + protocol - + "," + startPort - + "," + endPort - + ",[" + accountGroupNames + return "ruleGroupMatches(protocol:" + protocol + + ",startPort:" + startPort + + ",endPort:" + endPort + + ",accountGroupNames:[" + accountGroupNames + "])"; } }; diff --git a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionLiveTest.java b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionLiveTest.java index 50bd968871..973c164e54 100644 --- a/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionLiveTest.java +++ b/apis/cloudstack/src/test/java/org/jclouds/cloudstack/compute/extensions/CloudStackSecurityGroupExtensionLiveTest.java @@ -24,8 +24,6 @@ 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; /** @@ -48,14 +46,13 @@ public class CloudStackSecurityGroupExtensionLiveTest extends BaseSecurityGroupE super.setupContext(); CloudStackApi api = view.unwrapApi(CloudStackApi.class); - zone = Iterables.find(api.getZoneApi().listZones(), new Predicate() { - - @Override - public boolean apply(Zone arg0) { - return arg0.isSecurityGroupsEnabled(); + for (Zone z: api.getZoneApi().listZones()) { + if (z.isSecurityGroupsEnabled()) { + zone = z; + break; } + } - }); if (zone == null) securityGroupsSupported = false; }