From 3febe726a1882e09f5de2bdcd8f624a3683579aa Mon Sep 17 00:00:00 2001 From: Adrian Cole Date: Mon, 8 Oct 2012 15:39:51 -0400 Subject: [PATCH] Issue 671: NPE on aws-ec2 w/vpc security groups --- .../org/jclouds/ec2/domain/IpPermission.java | 143 ++++++++++- .../jclouds/ec2/domain/IpPermissionImpl.java | 226 ------------------ .../org/jclouds/ec2/domain/SecurityGroup.java | 204 +++++++++++----- .../org/jclouds/ec2/util/IpPermissions.java | 9 +- ...DescribeSecurityGroupsResponseHandler.java | 169 ++++++------- .../jclouds/ec2/xml/IpPermissionHandler.java | 75 ++++++ .../jclouds/ec2/xml/SecurityGroupHandler.java | 145 +++++++++++ ...ribeSecurityGroupsResponseHandlerTest.java | 20 +- .../resources/describe_securitygroups.xml | 71 +++--- .../describe_securitygroups_empty.xml | 70 +++--- .../DescribeSecurityGroupsResponseTest.java | 87 +++++++ .../describe_security_groups_vpc.xml | 35 +++ 12 files changed, 771 insertions(+), 483 deletions(-) delete mode 100644 apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermissionImpl.java create mode 100644 apis/ec2/src/main/java/org/jclouds/ec2/xml/IpPermissionHandler.java create mode 100644 apis/ec2/src/main/java/org/jclouds/ec2/xml/SecurityGroupHandler.java create mode 100644 providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/parse/DescribeSecurityGroupsResponseTest.java create mode 100644 providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermission.java b/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermission.java index 722313161c..d83991dff3 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermission.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermission.java @@ -18,49 +18,174 @@ */ package org.jclouds.ec2.domain; +import static com.google.common.base.Preconditions.checkNotNull; + import java.util.Set; +import com.google.common.base.Objects; +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; /** - * + * * @see * @author Adrian Cole */ -public interface IpPermission extends Comparable { +public class IpPermission { + public static Builder builder() { + return new Builder(); + } + + public static class Builder { + private int fromPort; + private int toPort; + private IpProtocol ipProtocol; + private Multimap userIdGroupPairs = LinkedHashMultimap.create(); + private Set groupIds = Sets.newLinkedHashSet(); + private Set ipRanges = Sets.newLinkedHashSet(); + + public Builder fromPort(int fromPort) { + this.fromPort = fromPort; + return this; + } + + public Builder toPort(int toPort) { + this.toPort = toPort; + return this; + } + + public Builder ipProtocol(IpProtocol ipProtocol) { + this.ipProtocol = checkNotNull(ipProtocol, "ipProtocol"); + return this; + } + + public Builder userIdGroupPair(String userId, String groupNameOrId) { + this.userIdGroupPairs.put(checkNotNull(userId, "userId"), checkNotNull(groupNameOrId, "groupNameOrId of %s", userId)); + return this; + } + + public Builder userIdGroupPairs(Multimap userIdGroupPairs) { + this.userIdGroupPairs.putAll(checkNotNull(userIdGroupPairs, "userIdGroupPairs")); + return this; + } + + public Builder ipRange(String ipRange) { + this.ipRanges.add(ipRange); + return this; + } + + public Builder ipRanges(Iterable ipRanges) { + Iterables.addAll(this.ipRanges, checkNotNull(ipRanges, "ipRanges")); + return this; + } + + public Builder groupId(String groupId) { + this.groupIds.add(checkNotNull(groupId, "groupId")); + return this; + } + + public Builder groupIds(Iterable groupIds) { + Iterables.addAll(this.groupIds, checkNotNull(groupIds, "groupIds")); + return this; + } + + public IpPermission build() { + return new IpPermission(ipProtocol, fromPort, toPort, userIdGroupPairs, groupIds, ipRanges); + } + } + + private final int fromPort; + private final int toPort; + private final Multimap userIdGroupPairs; + private final Set groupIds; + private final IpProtocol ipProtocol; + private final Set ipRanges; + + public IpPermission(IpProtocol ipProtocol, int fromPort, int toPort, Multimap userIdGroupPairs, + Iterable groupIds, Iterable ipRanges) { + this.fromPort = fromPort; + this.toPort = toPort; + this.userIdGroupPairs = ImmutableMultimap.copyOf(checkNotNull(userIdGroupPairs, "userIdGroupPairs")); + this.ipProtocol = checkNotNull(ipProtocol, "ipProtocol"); + this.groupIds = ImmutableSet.copyOf(checkNotNull(groupIds, "groupIds")); + this.ipRanges = ImmutableSet.copyOf(checkNotNull(ipRanges, "ipRanges")); + } /** * Start of port range for the TCP and UDP protocols, or an ICMP type number. * An ICMP type number of -1 indicates a wildcard (i.e., any ICMP type * number). */ - int getFromPort(); + public int getFromPort() { + return fromPort; + } /** * End of port range for the TCP and UDP protocols, or an ICMP code. An ICMP * code of -1 indicates a wildcard (i.e., any ICMP code). */ - int getToPort(); + public int getToPort() { + return toPort; + } /** * List of security group and user ID pairs. */ - Multimap getUserIdGroupPairs(); + public Multimap getUserIdGroupPairs() { + return userIdGroupPairs; + } /** * List of security group Ids */ - Set getGroupIds(); + public Set getGroupIds() { + return groupIds; + } /** * IP protocol */ - IpProtocol getIpProtocol(); + public IpProtocol getIpProtocol() { + return ipProtocol; + } /** * IP ranges. */ - Set getIpRanges(); -} \ No newline at end of file + public Set getIpRanges() { + return ipRanges; + } + + @Override + public int hashCode() { + return Objects.hashCode(fromPort, toPort, groupIds, ipProtocol, ipRanges, userIdGroupPairs); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null || getClass() != obj.getClass()) + return false; + IpPermission that = IpPermission.class.cast(obj); + return Objects.equal(this.fromPort, that.fromPort) && Objects.equal(this.toPort, that.toPort) + && Objects.equal(this.groupIds, that.groupIds) && Objects.equal(this.ipProtocol, that.ipProtocol) + && Objects.equal(this.ipRanges, that.ipRanges) + && Objects.equal(this.userIdGroupPairs, that.userIdGroupPairs); + } + + @Override + public String toString() { + return Objects.toStringHelper(this).add("fromPort", fromPort == -1 ? null : fromPort) + .add("toPort", toPort == -1 ? null : toPort).add("groupIds", groupIds.size() == 0 ? null : groupIds) + .add("ipProtocol", ipProtocol).add("ipRanges", ipRanges.size() == 0 ? null : ipRanges) + .add("userIdGroupPairs", userIdGroupPairs.size() == 0 ? null : userIdGroupPairs).toString(); + } + +} diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermissionImpl.java b/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermissionImpl.java deleted file mode 100644 index fe801981ee..0000000000 --- a/apis/ec2/src/main/java/org/jclouds/ec2/domain/IpPermissionImpl.java +++ /dev/null @@ -1,226 +0,0 @@ -/** - * Licensed to jclouds, Inc. (jclouds) under one or more - * contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. jclouds 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.ec2.domain; - -import static com.google.common.base.Preconditions.checkNotNull; - -import java.util.Set; - -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; - -/** - * - * @see - * @author Adrian Cole - */ -public class IpPermissionImpl implements IpPermission { - public static Builder builder() { - return new Builder(); - } - - public static class Builder { - private int fromPort; - private int toPort; - private IpProtocol ipProtocol; - private Multimap userIdGroupPairs = LinkedHashMultimap.create(); - private Set groupIds = Sets.newLinkedHashSet(); - private Set ipRanges = Sets.newLinkedHashSet(); - - public Builder fromPort(int fromPort) { - this.fromPort = fromPort; - return this; - } - - public Builder toPort(int toPort) { - this.fromPort = toPort; - return this; - } - - public Builder ipProtocol(IpProtocol ipProtocol) { - this.ipProtocol = ipProtocol; - return this; - } - - public Builder userIdGroupPair(String userId, String groupNameOrId) { - this.userIdGroupPairs.put(userId, groupNameOrId); - return this; - } - - public Builder userIdGroupPairs(Multimap userIdGroupPairs) { - this.userIdGroupPairs.putAll(userIdGroupPairs); - return this; - } - - public Builder ipRange(String ipRange) { - this.ipRanges.add(ipRange); - return this; - } - - public Builder ipRanges(Iterable ipRanges) { - Iterables.addAll(this.ipRanges, ipRanges); - return this; - } - - public Builder groupId(String groupId) { - this.groupIds.add(groupId); - return this; - } - - public Builder groupIds(Iterable groupIds) { - Iterables.addAll(this.groupIds, groupIds); - return this; - } - - public IpPermission build() { - return new IpPermissionImpl(ipProtocol, fromPort, toPort, userIdGroupPairs, groupIds, ipRanges); - } - } - - private final int fromPort; - private final int toPort; - private final Multimap userIdGroupPairs; - private final Set groupIds; - private final IpProtocol ipProtocol; - private final Set ipRanges; - - public IpPermissionImpl(IpProtocol ipProtocol, int fromPort, int toPort, - Multimap userIdGroupPairs, Iterable groupIds, Iterable ipRanges) { - this.fromPort = fromPort; - this.toPort = toPort; - this.userIdGroupPairs = ImmutableMultimap.copyOf(checkNotNull(userIdGroupPairs, "userIdGroupPairs")); - this.ipProtocol = checkNotNull(ipProtocol, "ipProtocol"); - this.groupIds = ImmutableSet.copyOf(checkNotNull(groupIds, "groupIds")); - this.ipRanges = ImmutableSet.copyOf(checkNotNull(ipRanges, "ipRanges")); - } - - /** - * {@inheritDoc} - */ - public int compareTo(IpPermission o) { - return (this == o) ? 0 : getIpProtocol().compareTo(o.getIpProtocol()); - } - - /** - * {@inheritDoc} - */ - @Override - public int getFromPort() { - return fromPort; - } - - /** - * {@inheritDoc} - */ - @Override - public int getToPort() { - return toPort; - } - - /** - * {@inheritDoc} - */ - @Override - public Multimap getUserIdGroupPairs() { - return userIdGroupPairs; - } - - /** - * {@inheritDoc} - */ - @Override - public Set getGroupIds() { - return groupIds; - } - - /** - * {@inheritDoc} - */ - @Override - public IpProtocol getIpProtocol() { - return ipProtocol; - } - - /** - * {@inheritDoc} - */ - @Override - public Set getIpRanges() { - return ipRanges; - } - - @Override - public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + fromPort; - result = prime * result + ((groupIds == null) ? 0 : groupIds.hashCode()); - result = prime * result + ((ipProtocol == null) ? 0 : ipProtocol.hashCode()); - result = prime * result + ((ipRanges == null) ? 0 : ipRanges.hashCode()); - result = prime * result + toPort; - result = prime * result + ((userIdGroupPairs == null) ? 0 : userIdGroupPairs.hashCode()); - return result; - } - - @Override - public boolean equals(Object obj) { - if (this == obj) - return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - IpPermissionImpl other = (IpPermissionImpl) obj; - if (fromPort != other.fromPort) - return false; - if (groupIds == null) { - if (other.groupIds != null) - return false; - } else if (!groupIds.equals(other.groupIds)) - return false; - if (ipProtocol != other.ipProtocol) - return false; - if (ipRanges == null) { - if (other.ipRanges != null) - return false; - } else if (!ipRanges.equals(other.ipRanges)) - return false; - if (toPort != other.toPort) - return false; - if (userIdGroupPairs == null) { - if (other.userIdGroupPairs != null) - return false; - } else if (!userIdGroupPairs.equals(other.userIdGroupPairs)) - return false; - return true; - } - - @Override - public String toString() { - return "[fromPort=" + fromPort + ", toPort=" + toPort + ", userIdGroupPairs=" + userIdGroupPairs + ", groupIds=" - + groupIds + ", ipProtocol=" + ipProtocol + ", ipRanges=" + ipRanges + "]"; - } - -} diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/domain/SecurityGroup.java b/apis/ec2/src/main/java/org/jclouds/ec2/domain/SecurityGroup.java index a0a6742b09..15eeab8a91 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/domain/SecurityGroup.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/domain/SecurityGroup.java @@ -18,55 +18,153 @@ */ package org.jclouds.ec2.domain; +import static com.google.common.base.Preconditions.checkNotNull; + import java.util.Set; import org.jclouds.javax.annotation.Nullable; -import static com.google.common.base.Preconditions.checkNotNull; +import com.google.common.base.Objects; +import com.google.common.base.Objects.ToStringHelper; +import com.google.common.collect.ForwardingSet; +import com.google.common.collect.ImmutableSet; /** - * + * * @see * @author Adrian Cole */ -public class SecurityGroup implements Comparable { +public class SecurityGroup extends ForwardingSet { + + public static Builder builder() { + return new ConcreteBuilder(); + } + + public Builder toBuilder() { + return new ConcreteBuilder().fromSecurityGroup(this); + } + + public static abstract class Builder> { + protected abstract T self(); + + protected String region; + protected String id; + protected String name; + protected String ownerId; + protected String description; + protected ImmutableSet.Builder ipPermissions = ImmutableSet. builder(); + + /** + * @see SecurityGroup#getRegion() + */ + public T region(String region) { + this.region = region; + return self(); + } + + /** + * @see SecurityGroup#getId() + */ + public T id(String id) { + this.id = id; + return self(); + } + + /** + * @see SecurityGroup#getName() + */ + public T name(String name) { + this.name = name; + return self(); + } + + /** + * @see SecurityGroup#getOwnerId() + */ + public T ownerId(String ownerId) { + this.ownerId = ownerId; + return self(); + } + + /** + * @see SecurityGroup#getDescription() + */ + public T description(String description) { + this.description = description; + return self(); + } + + /** + * @see SecurityGroup#delegate() + */ + public T role(IpPermission role) { + this.ipPermissions.add(role); + return self(); + } + + /** + * @see SecurityGroup#delegate() + */ + public T ipPermissions(Iterable ipPermissions) { + this.ipPermissions.addAll(checkNotNull(ipPermissions, "ipPermissions")); + return self(); + } + + /** + * @see SecurityGroup#delegate() + */ + public T ipPermission(IpPermission ipPermission) { + this.ipPermissions.add(checkNotNull(ipPermission, "ipPermission")); + return self(); + } + + public SecurityGroup build() { + return new SecurityGroup(region, id, name, ownerId, description, ipPermissions.build()); + } + + public T fromSecurityGroup(SecurityGroup in) { + return region(in.region).id(in.id).name(in.name).ownerId(in.ownerId).description(in.description) + .ipPermissions(in); + } + } + + private static class ConcreteBuilder extends Builder { + @Override + protected ConcreteBuilder self() { + return this; + } + } private final String region; private final String id; private final String name; private final String ownerId; private final String description; - private final Set ipPermissions; + private final Set ipPermissions; public SecurityGroup(String region, String id, String name, String ownerId, String description, - Set ipPermissions) { + Iterable ipPermissions) { this.region = checkNotNull(region, "region"); this.id = id; this.name = name; this.ownerId = ownerId; this.description = description; - this.ipPermissions = ipPermissions; + this.ipPermissions = ImmutableSet.copyOf(checkNotNull(ipPermissions, "ipPermissions")); } /** - * Security groups are not copied across Regions. Instances within the Region - * cannot communicate with instances outside the Region using group-based - * firewall rules. Traffic from instances in another Region is seen as WAN - * bandwidth. + * To be removed in jclouds 1.6

Warning

+ * + * Especially on EC2 clones that may not support regions, this value is + * fragile. Consider alternate means to determine context. */ + @Deprecated public String getRegion() { return region; } - /** - * {@inheritDoc} - */ - public int compareTo(SecurityGroup o) { - return (this == o) ? 0 : getName().compareTo(o.getName()); - } - /** * id of the security group. Not in all EC2 impls */ @@ -97,70 +195,46 @@ public class SecurityGroup implements Comparable { } /** - * Set of IP permissions associated with the security group. + * Please use this class as a collection */ - public Set getIpPermissions() { + @Deprecated + public Set getIpPermissions() { return ipPermissions; } @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((description == null) ? 0 : description.hashCode()); - result = prime * result + ((id == null) ? 0 : id.hashCode()); - result = prime * result + ((ipPermissions == null) ? 0 : ipPermissions.hashCode()); - result = prime * result + ((name == null) ? 0 : name.hashCode()); - result = prime * result + ((ownerId == null) ? 0 : ownerId.hashCode()); - result = prime * result + ((region == null) ? 0 : region.hashCode()); - return result; + return Objects.hashCode(region, id, name, ownerId, description, ipPermissions); } @Override public boolean equals(Object obj) { if (this == obj) return true; - if (obj == null) + if (obj == null || getClass() != obj.getClass()) return false; - if (getClass() != obj.getClass()) - return false; - SecurityGroup other = (SecurityGroup) obj; - if (description == null) { - if (other.description != null) - return false; - } else if (!description.equals(other.description)) - return false; - if (id == null) { - if (other.id != null) - return false; - } else if (!id.equals(other.id)) - return false; - if (ipPermissions == null) { - if (other.ipPermissions != null) - return false; - } else if (!ipPermissions.equals(other.ipPermissions)) - return false; - if (name == null) { - if (other.name != null) - return false; - } else if (!name.equals(other.name)) - return false; - if (ownerId == null) { - if (other.ownerId != null) - return false; - } else if (!ownerId.equals(other.ownerId)) - return false; - if (region == null) { - if (other.region != null) - return false; - } else if (!region.equals(other.region)) - return false; - return true; + SecurityGroup that = SecurityGroup.class.cast(obj); + return Objects.equal(this.region, that.region) + && Objects.equal(this.id, that.id) + && Objects.equal(this.name, that.name) + && Objects.equal(this.ownerId, that.ownerId) + && Objects.equal(this.description, that.description) + && Objects.equal(this.ipPermissions, that.ipPermissions); + } + + protected ToStringHelper string() { + return Objects.toStringHelper(this).add("region", region).add("id", id).add("name", name) + .add("ownerId", ownerId).add("description", description) + .add("ipPermissions", ipPermissions.size() == 0 ? null : ipPermissions); } @Override public String toString() { - return "[region=" + region + ", id=" + id + ", name=" + name + ", ownerId=" + ownerId + ", description=" - + description + ", ipPermissions=" + ipPermissions + "]"; + return string().toString(); + } + + @Override + protected Set delegate() { + return ipPermissions; } } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/util/IpPermissions.java b/apis/ec2/src/main/java/org/jclouds/ec2/util/IpPermissions.java index d14cc7fe67..fb5a331b89 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/util/IpPermissions.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/util/IpPermissions.java @@ -24,7 +24,6 @@ import java.util.Map; import java.util.Map.Entry; import org.jclouds.ec2.domain.IpPermission; -import org.jclouds.ec2.domain.IpPermissionImpl; import org.jclouds.ec2.domain.IpProtocol; import org.jclouds.util.Maps2; @@ -37,12 +36,12 @@ import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; /** - * + * * Shortcut to create ingress rules - * + * * @author Adrian Cole */ -public class IpPermissions extends IpPermissionImpl { +public class IpPermissions extends IpPermission { protected IpPermissions(IpProtocol ipProtocol, int fromPort, int toPort, Multimap userIdGroupPairs, Iterable groupIds, Iterable ipRanges) { @@ -138,7 +137,7 @@ public class IpPermissions extends IpPermissionImpl { public ToPortSelection fromPort(int port) { return new ToPortSelection(getIpProtocol(), port); } - + public ToSourceSelection port(int port) { return new ToSourceSelection(getIpProtocol(), port, port); } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandler.java b/apis/ec2/src/main/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandler.java index 9b1b53ddf9..3db80b9471 100644 --- a/apis/ec2/src/main/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandler.java +++ b/apis/ec2/src/main/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandler.java @@ -18,138 +18,111 @@ */ package org.jclouds.ec2.xml; -import static org.jclouds.util.SaxUtils.currentOrNegative; -import static org.jclouds.util.SaxUtils.currentOrNull; import static org.jclouds.util.SaxUtils.equalsOrSuffix; import java.util.Set; import javax.inject.Inject; -import org.jclouds.aws.util.AWSUtils; -import org.jclouds.ec2.domain.IpPermissionImpl; -import org.jclouds.ec2.domain.IpProtocol; import org.jclouds.ec2.domain.SecurityGroup; +import org.jclouds.http.HttpRequest; import org.jclouds.http.functions.ParseSax; -import org.jclouds.location.Region; +import org.jclouds.http.functions.ParseSax.HandlerForGeneratedRequestWithResult; import org.xml.sax.Attributes; +import org.xml.sax.SAXException; -import com.google.common.base.Supplier; import com.google.common.collect.ImmutableSet; -import com.google.common.collect.LinkedHashMultimap; -import com.google.common.collect.Multimap; -import com.google.common.collect.Sets; +import com.google.common.collect.ImmutableSet.Builder; /** * Parses: DescribeSecurityGroupsResponse * xmlns="http://ec2.amazonaws.com/doc/2010-06-15/" - * + * * @see * @author Adrian Cole */ public class DescribeSecurityGroupsResponseHandler extends ParseSax.HandlerForGeneratedRequestWithResult> { - @Inject - @Region - Supplier defaultRegion; + + private final SecurityGroupHandler securityGroupHandler; private StringBuilder currentText = new StringBuilder(); - private Set securtyGroups = Sets.newLinkedHashSet(); - private String groupId; - private String groupName; - private String ownerId; - private String groupDescription; - private Set ipPermissions = Sets.newLinkedHashSet(); - private int fromPort; - private int toPort; - private Multimap groups = LinkedHashMultimap.create(); - private String userId; - private String userIdGroupName; - private IpProtocol ipProtocol; - private Set ipRanges = Sets.newLinkedHashSet(); + private Builder securityGroups = ImmutableSet. builder(); + private boolean inSecurityGroupInfo; - private boolean inIpPermissions; - private boolean inIpRanges; - private boolean inGroups; + protected int itemDepth; + @Inject + public DescribeSecurityGroupsResponseHandler(SecurityGroupHandler securityGroupHandler) { + this.securityGroupHandler = securityGroupHandler; + } + + @Override + public HandlerForGeneratedRequestWithResult> setContext(HttpRequest request) { + securityGroupHandler.setContext(request); + return super.setContext(request); + } + + /** + * {@inheritDoc} + */ + @Override public Set getResult() { - return securtyGroups; + return securityGroups.build(); } - public void startElement(String uri, String name, String qName, Attributes attrs) { - if (equalsOrSuffix(qName, "ipPermissions")) { - inIpPermissions = true; - } else if (equalsOrSuffix(qName, "ipRanges")) { - inIpRanges = true; - } else if (equalsOrSuffix(qName, "groups")) { - inGroups = true; + /** + * {@inheritDoc} + */ + @Override + public void startElement(String url, String name, String qName, Attributes attributes) throws SAXException { + if (equalsOrSuffix(qName, "item")) { + itemDepth++; + } else if (equalsOrSuffix(qName, "securityGroupInfo")) { + inSecurityGroupInfo = true; + } + if (inSecurityGroupInfo) { + securityGroupHandler.startElement(url, name, qName, attributes); } } - public void endElement(String uri, String name, String qName) { - if (equalsOrSuffix(qName, "groupName")) { - if (!inGroups) - this.groupName = currentOrNull(currentText); - else - this.userIdGroupName = currentOrNull(currentText); - } else if (equalsOrSuffix(qName, "groupId")) { - this.groupId = currentOrNull(currentText); - } else if (equalsOrSuffix(qName, "ownerId")) { - this.ownerId = currentOrNull(currentText); - } else if (equalsOrSuffix(qName, "userId")) { - this.userId = currentOrNull(currentText); - } else if (equalsOrSuffix(qName, "groupDescription")) { - this.groupDescription = currentOrNull(currentText); - } else if (equalsOrSuffix(qName, "ipProtocol")) { - // Algorete: ipProtocol can be an empty tag on EC2 clone (e.g. OpenStack EC2) - this.ipProtocol = IpProtocol.fromValue(currentOrNegative(currentText)); - } else if (equalsOrSuffix(qName, "fromPort")) { - // Algorete: fromPort can be an empty tag on EC2 clone (e.g. OpenStack EC2) - this.fromPort = Integer.parseInt(currentOrNegative(currentText)); - } else if (equalsOrSuffix(qName, "toPort")) { - // Algorete: toPort can be an empty tag on EC2 clone (e.g. OpenStack EC2) - this.toPort = Integer.parseInt(currentOrNegative(currentText)); - } else if (equalsOrSuffix(qName, "cidrIp")) { - this.ipRanges.add(currentOrNull(currentText)); - } else if (equalsOrSuffix(qName, "ipPermissions")) { - inIpPermissions = false; - } else if (equalsOrSuffix(qName, "ipRanges")) { - inIpRanges = false; - } else if (equalsOrSuffix(qName, "groups")) { - inGroups = false; - } else if (equalsOrSuffix(qName, "item")) { - if (inIpPermissions && !inIpRanges && !inGroups) { - // TODO groups? we need an example of VPC stuff - ipPermissions.add(new IpPermissionImpl(ipProtocol, fromPort, toPort, groups, ImmutableSet. of(), - ipRanges)); - this.fromPort = -1; - this.toPort = -1; - this.groups = LinkedHashMultimap.create(); - this.ipProtocol = null; - this.ipRanges = Sets.newLinkedHashSet(); - } else if (inIpPermissions && !inIpRanges && inGroups) { - this.groups.put(userId, userIdGroupName); - this.userId = null; - this.userIdGroupName = null; - } else if (!inIpPermissions && !inIpRanges && !inGroups) { - String region = AWSUtils.findRegionInArgsOrNull(getRequest()); - if (region == null) - region = defaultRegion.get(); - securtyGroups.add(new SecurityGroup(region, groupId, groupName, ownerId, groupDescription, ipPermissions)); - this.groupName = null; - this.groupId = null; - this.ownerId = null; - this.groupDescription = null; - this.ipPermissions = Sets.newLinkedHashSet(); - } + /** + * {@inheritDoc} + */ + @Override + public void endElement(String uri, String name, String qName) throws SAXException { + if (equalsOrSuffix(qName, "item")) { + endItem(uri, name, qName); + itemDepth--; + } else if (equalsOrSuffix(qName, "securityGroupInfo")) { + inSecurityGroupInfo = false; + } else if (inSecurityGroupInfo) { + securityGroupHandler.endElement(uri, name, qName); } - currentText = new StringBuilder(); } - public void characters(char ch[], int start, int length) { - currentText.append(ch, start, length); + protected void endItem(String uri, String name, String qName) throws SAXException { + if (inSecurityGroupInfo) { + if (itemDepth == 1) + securityGroups.add(securityGroupHandler.getResult()); + else + securityGroupHandler.endElement(uri, name, qName); + } } + + /** + * {@inheritDoc} + */ + @Override + public void characters(char ch[], int start, int length) { + if (inSecurityGroupInfo) { + securityGroupHandler.characters(ch, start, length); + } else { + currentText.append(ch, start, length); + } + } + } diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/xml/IpPermissionHandler.java b/apis/ec2/src/main/java/org/jclouds/ec2/xml/IpPermissionHandler.java new file mode 100644 index 0000000000..8045feee40 --- /dev/null +++ b/apis/ec2/src/main/java/org/jclouds/ec2/xml/IpPermissionHandler.java @@ -0,0 +1,75 @@ +package org.jclouds.ec2.xml; + +import static org.jclouds.util.SaxUtils.currentOrNegative; +import static org.jclouds.util.SaxUtils.currentOrNull; +import static org.jclouds.util.SaxUtils.equalsOrSuffix; + +import org.jclouds.ec2.domain.IpPermission; +import org.jclouds.ec2.domain.IpProtocol; +import org.jclouds.http.functions.ParseSax; +import org.xml.sax.SAXException; + +/** + * + * @author Adrian Cole + */ +public class IpPermissionHandler extends ParseSax.HandlerForGeneratedRequestWithResult { + + private StringBuilder currentText = new StringBuilder(); + private IpPermission.Builder builder = IpPermission.builder(); + + /** + * {@inheritDoc} + */ + @Override + public IpPermission getResult() { + try { + return builder.build(); + } finally { + builder = IpPermission.builder(); + } + } + + private String userId; + private String groupId; + + /** + * {@inheritDoc} + */ + @Override + public void endElement(String uri, String name, String qName) throws SAXException { + if (equalsOrSuffix(qName, "ipProtocol")) { + // Algorete: ipProtocol can be an empty tag on EC2 clone (e.g. + // OpenStack EC2) + builder.ipProtocol(IpProtocol.fromValue(currentOrNegative(currentText))); + } else if (equalsOrSuffix(qName, "fromPort")) { + // Algorete: fromPort can be an empty tag on EC2 clone (e.g. OpenStack + // EC2) + builder.fromPort(Integer.parseInt(currentOrNegative(currentText))); + } else if (equalsOrSuffix(qName, "toPort")) { + // Algorete: toPort can be an empty tag on EC2 clone (e.g. OpenStack + // EC2) + builder.toPort(Integer.parseInt(currentOrNegative(currentText))); + } else if (equalsOrSuffix(qName, "cidrIp")) { + builder.ipRange(currentOrNull(currentText)); + } else if (equalsOrSuffix(qName, "userId")) { + this.userId = currentOrNull(currentText); + } else if (equalsOrSuffix(qName, "groupName") || equalsOrSuffix(qName, "groupId")) { + this.groupId = currentOrNull(currentText); + } else if (equalsOrSuffix(qName, "item")) { + if (userId != null && groupId != null) + builder.userIdGroupPair(userId, groupId); + userId = groupId = null; + } + currentText = new StringBuilder(); + } + + /** + * {@inheritDoc} + */ + @Override + public void characters(char ch[], int start, int length) { + currentText.append(ch, start, length); + } + +} diff --git a/apis/ec2/src/main/java/org/jclouds/ec2/xml/SecurityGroupHandler.java b/apis/ec2/src/main/java/org/jclouds/ec2/xml/SecurityGroupHandler.java new file mode 100644 index 0000000000..aeb7dbdec0 --- /dev/null +++ b/apis/ec2/src/main/java/org/jclouds/ec2/xml/SecurityGroupHandler.java @@ -0,0 +1,145 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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.ec2.xml; + +import static org.jclouds.util.SaxUtils.currentOrNull; +import static org.jclouds.util.SaxUtils.equalsOrSuffix; + +import org.jclouds.aws.util.AWSUtils; +import org.jclouds.ec2.domain.SecurityGroup; +import org.jclouds.http.HttpRequest; +import org.jclouds.http.functions.ParseSax; +import org.jclouds.http.functions.ParseSax.HandlerForGeneratedRequestWithResult; +import org.jclouds.location.Region; +import org.jclouds.rest.internal.GeneratedHttpRequest; +import org.xml.sax.Attributes; +import org.xml.sax.SAXException; + +import com.google.common.base.Supplier; +import com.google.inject.Inject; + +/** + * @author Adrian Cole + */ +public class SecurityGroupHandler extends ParseSax.HandlerForGeneratedRequestWithResult { + + protected final IpPermissionHandler ipPermissionHandler; + protected final Supplier defaultRegion; + + protected StringBuilder currentText = new StringBuilder(); + protected SecurityGroup.Builder builder; + protected boolean inIpPermissions; + + protected int itemDepth; + + protected String region; + + @Inject + public SecurityGroupHandler(IpPermissionHandler ipPermissionHandler, @Region Supplier defaultRegion) { + this.ipPermissionHandler = ipPermissionHandler; + this.defaultRegion = defaultRegion; + } + + protected SecurityGroup.Builder builder() { + return SecurityGroup.builder().region(region); + } + + @Override + public HandlerForGeneratedRequestWithResult setContext(HttpRequest request) { + region = AWSUtils.findRegionInArgsOrNull(GeneratedHttpRequest.class.cast(request)); + if (region == null) + region = defaultRegion.get(); + builder = builder(); + return super.setContext(request); + } + + /** + * {@inheritDoc} + */ + @Override + public SecurityGroup getResult() { + try { + return builder.build(); + } finally { + builder = builder(); + } + } + + /** + * {@inheritDoc} + */ + @Override + public void startElement(String url, String name, String qName, Attributes attributes) throws SAXException { + if (equalsOrSuffix(qName, "item")) { + itemDepth++; + } else if (equalsOrSuffix(qName, "ipPermissions")) { + inIpPermissions = true; + } + if (inIpPermissions) { + ipPermissionHandler.startElement(url, name, qName, attributes); + } + } + + /** + * {@inheritDoc} + */ + @Override + public void endElement(String uri, String name, String qName) throws SAXException { + if (equalsOrSuffix(qName, "item")) { + endItem(uri, name, qName); + itemDepth--; + } else if (equalsOrSuffix(qName, "ipPermissions")) { + inIpPermissions = false; + itemDepth = 0; + } else if (inIpPermissions) { + ipPermissionHandler.endElement(uri, name, qName); + } else if (equalsOrSuffix(qName, "groupName")) { + builder.name(currentOrNull(currentText)); + } else if (equalsOrSuffix(qName, "groupId")) { + builder.id(currentOrNull(currentText)); + } else if (equalsOrSuffix(qName, "ownerId")) { + builder.ownerId(currentOrNull(currentText)); + } else if (equalsOrSuffix(qName, "groupDescription")) { + builder.description(currentOrNull(currentText)); + } + currentText = new StringBuilder(); + } + + protected void endItem(String uri, String name, String qName) throws SAXException { + if (inIpPermissions) { + if (itemDepth == 2) + builder.ipPermission(ipPermissionHandler.getResult()); + else + ipPermissionHandler.endElement(uri, name, qName); + } + } + + /** + * {@inheritDoc} + */ + @Override + public void characters(char ch[], int start, int length) { + if (inIpPermissions) { + ipPermissionHandler.characters(ch, start, length); + } else { + currentText.append(ch, start, length); + } + } + +} diff --git a/apis/ec2/src/test/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandlerTest.java b/apis/ec2/src/test/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandlerTest.java index c85158b7a5..01276fdc13 100644 --- a/apis/ec2/src/test/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandlerTest.java +++ b/apis/ec2/src/test/java/org/jclouds/ec2/xml/DescribeSecurityGroupsResponseHandlerTest.java @@ -26,7 +26,7 @@ import static org.testng.Assert.assertEquals; import java.io.InputStream; import java.util.Set; -import org.jclouds.ec2.domain.IpPermissionImpl; +import org.jclouds.ec2.domain.IpPermission; import org.jclouds.ec2.domain.IpProtocol; import org.jclouds.ec2.domain.SecurityGroup; import org.jclouds.http.functions.ParseSax; @@ -41,7 +41,7 @@ import com.google.common.collect.Multimap; /** * Tests behavior of {@code DescribeSecurityGroupsResponseHandler} - * + * * @author Adrian Cole */ // NOTE:without testName, this will not call @Before* and fail w/NPE during @@ -54,40 +54,40 @@ public class DescribeSecurityGroupsResponseHandlerTest extends BaseEC2HandlerTes Set expected = ImmutableSet.of( new SecurityGroup(defaultRegion, null, "WebServers", "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM", "Web Servers", - ImmutableSet.of(new IpPermissionImpl(IpProtocol.TCP, 80, 80, ImmutableMultimap. of(), + ImmutableSet.of(new IpPermission(IpProtocol.TCP, 80, 80, ImmutableMultimap. of(), ImmutableSet. of(), ImmutableSet.of("0.0.0.0/0")))), new SecurityGroup(defaultRegion, null, "RangedPortsBySource", "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM", "Group A", - ImmutableSet.of(new IpPermissionImpl(IpProtocol.TCP, 6000, 7000, ImmutableMultimap + ImmutableSet.of(new IpPermission(IpProtocol.TCP, 6000, 7000, ImmutableMultimap . of(), ImmutableSet. of(), ImmutableSet. of())))); DescribeSecurityGroupsResponseHandler handler = injector.getInstance(DescribeSecurityGroupsResponseHandler.class); addDefaultRegionToHandler(handler); Set result = factory.create(handler).parse(is); - assertEquals(result, expected); + assertEquals(result.toString(), expected.toString()); } // Response from OpenStack 1.1 EC2 API public void testApplyInputStreamWithEmptyFields() { InputStream is = getClass().getResourceAsStream("/describe_securitygroups_empty.xml"); - + Multimap userIdGroupPairs = LinkedHashMultimap.create(); userIdGroupPairs.put("UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM", "jclouds#cluster#world"); - + Set expected = ImmutableSet.of( new SecurityGroup(defaultRegion, null, "jclouds#cluster#world", "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM", "Cluster", ImmutableSet.of( - new IpPermissionImpl(IpProtocol.TCP, 22, 22, ImmutableMultimap. of(), + new IpPermission(IpProtocol.TCP, 22, 22, ImmutableMultimap. of(), ImmutableSet. of(), ImmutableSet.of("0.0.0.0/0")), - new IpPermissionImpl(IpProtocol.ALL, -1, -1, userIdGroupPairs, + new IpPermission(IpProtocol.ALL, -1, -1, userIdGroupPairs, ImmutableSet. of(), ImmutableSet. of())))); DescribeSecurityGroupsResponseHandler handler = injector.getInstance(DescribeSecurityGroupsResponseHandler.class); addDefaultRegionToHandler(handler); Set result = factory.create(handler).parse(is); - assertEquals(result, expected); + assertEquals(result.toString(), expected.toString()); } private void addDefaultRegionToHandler(ParseSax.HandlerWithResult handler) { diff --git a/apis/ec2/src/test/resources/describe_securitygroups.xml b/apis/ec2/src/test/resources/describe_securitygroups.xml index 5d6087f546..3a6c9b86d3 100644 --- a/apis/ec2/src/test/resources/describe_securitygroups.xml +++ b/apis/ec2/src/test/resources/describe_securitygroups.xml @@ -1,36 +1,37 @@ - - - - UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM - WebServers - Web Servers - - - tcp - 80 - 80 - - - - 0.0.0.0/0 - - - - - - - UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM - RangedPortsBySource - Group A - - - tcp - 6000 - 7000 - - - - - - + + + + UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM + WebServers + Web Servers + + + tcp + 80 + 80 + + + + 0.0.0.0/0 + + + + + + + UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM + RangedPortsBySource + Group A + + + tcp + 6000 + 7000 + + + + + + \ No newline at end of file diff --git a/apis/ec2/src/test/resources/describe_securitygroups_empty.xml b/apis/ec2/src/test/resources/describe_securitygroups_empty.xml index 75add413cd..0079ee6abd 100644 --- a/apis/ec2/src/test/resources/describe_securitygroups_empty.xml +++ b/apis/ec2/src/test/resources/describe_securitygroups_empty.xml @@ -1,39 +1,39 @@ - - L6EFIZVPJS76T3K5-0UV - + + L6EFIZVPJS76T3K5-0UV + + + + + 22 + tcp + + + 0.0.0.0/0 + + + + 22 + - - - - 22 - tcp - - - 0.0.0.0/0 - - - - 22 - + + + + + + + jclouds#cluster#world + UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM + + + + + + jclouds#cluster#world + Cluster + UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM + - - - - - - - jclouds#cluster#world - UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM - - - - - - jclouds#cluster#world - Cluster - UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM - - - + diff --git a/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/parse/DescribeSecurityGroupsResponseTest.java b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/parse/DescribeSecurityGroupsResponseTest.java new file mode 100644 index 0000000000..0394d2baae --- /dev/null +++ b/providers/aws-ec2/src/test/java/org/jclouds/aws/ec2/parse/DescribeSecurityGroupsResponseTest.java @@ -0,0 +1,87 @@ +/** + * Licensed to jclouds, Inc. (jclouds) under one or more + * contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. jclouds 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.aws.ec2.parse; + +import static org.easymock.EasyMock.createMock; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.testng.Assert.assertEquals; + +import java.io.InputStream; +import java.util.Set; + +import org.jclouds.ec2.domain.IpPermission; +import org.jclouds.ec2.domain.IpProtocol; +import org.jclouds.ec2.domain.SecurityGroup; +import org.jclouds.ec2.xml.BaseEC2HandlerTest; +import org.jclouds.ec2.xml.DescribeSecurityGroupsResponseHandler; +import org.jclouds.http.functions.ParseSax; +import org.jclouds.rest.internal.GeneratedHttpRequest; +import org.testng.annotations.Test; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; + +/** + * + * @author Adrian Cole + */ +// NOTE:without testName, this will not call @Before* and fail w/NPE during +// surefire +@Test(groups = "unit", testName = "DescribeSecurityGroupsResponseTest") +public class DescribeSecurityGroupsResponseTest extends BaseEC2HandlerTest { + + public void test() { + InputStream is = getClass().getResourceAsStream("/describe_security_groups_vpc.xml"); + + Set expected = expected(); + + DescribeSecurityGroupsResponseHandler handler = injector.getInstance(DescribeSecurityGroupsResponseHandler.class); + addDefaultRegionToHandler(handler); + Set result = factory.create(handler).parse(is); + + assertEquals(result.toString(), expected.toString()); + } + + public Set expected() { + return ImmutableSet.of(SecurityGroup.builder() + .region(defaultRegion) + .ownerId("123123123123") + .id("sg-11111111") + .name("default") + .description("default VPC security group") +// .vpcId("vpc-99999999") + .ipPermission(IpPermission.builder() + .ipProtocol(IpProtocol.ALL) + .userIdGroupPair("123123123123","sg-11111111").build()) +// .ipPermissionEgress(IpPermission.builder() +// .ipProtocol(IpProtocol.ALL) +// .ipRange("0.0.0.0/0").build()) + .build()); + + } + + private void addDefaultRegionToHandler(ParseSax.HandlerWithResult handler) { + GeneratedHttpRequest request = createMock(GeneratedHttpRequest.class); + expect(request.getArgs()).andReturn(ImmutableList. of()).atLeastOnce(); + replay(request); + handler.setContext(request); + } + +} diff --git a/providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml b/providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml new file mode 100644 index 0000000000..cd635b4e71 --- /dev/null +++ b/providers/aws-ec2/src/test/resources/describe_security_groups_vpc.xml @@ -0,0 +1,35 @@ + + xxxxxxxxxxxxxxxx + + + 123123123123 + sg-11111111 + default + default VPC security group + vpc-99999999 + + + -1 + + + 123123123123 + sg-11111111 + + + + + + + + -1 + + + + 0.0.0.0/0 + + + + + + + \ No newline at end of file