Merge lp:~oubiwann/txaws/432540-more-securitygroup-methods into lp:txaws

[Duncan McGreggor]

[Duncan McGreggor (oubiwann)]

This one's ready for review. This branch finishes the security group support for txAWS.

Note that I've added 4 convenience functions, two each for revoking and authorizing groups. This is to provide a slightly more intuitive API than what is offered by AWS EC2. The AWS EC2 methods have also been implemented, so nothing's missing. These are just more clear to use, since they split the EC2 methods into distinct areas of functionality (both the authorize and revoke AWS EC2 methods serve dual purpose: they modify a group with another group that has IP permissions or they modify a group with IP permissions).

[lp:~oubiwann/txaws/432540-more-securitygroup-methods]

31. By Duncan McGreggor on 2009-09-20

Added missing newline.

[Thomas Herve (therve)]

[1]
+ if success and success.lower() == "true":
+ return True
+ return False

I'm not sure you need to check for the presence of success. At any rate, you could do that:

return (success is not None and success.lower() == "true")

But I would do that:

return root.findtext("return") == "true"

It would handle less cases, but it would like to see it fails when EC2 is not respecting the format of the response (we don't have to be defensive here).

[2]
+ @return: A C{Deferred} that will fire with a turth value for the

typo: truth (in 2 places).

[3] Nice factoring with _parse_truth_return. Can you use it in the other places where similar thing is done?

+1!

review: Approve

[Duncan McGreggor (oubiwann)]

> [1]
> + if success and success.lower() == "true":
> + return True
> + return False
>
> I'm not sure you need to check for the presence of success. At any rate, you
> could do that:
>
> return (success is not None and success.lower() == "true")
>
> But I would do that:
>
> return root.findtext("return") == "true"
>
> It would handle less cases, but it would like to see it fails when EC2 is not
> respecting the format of the response (we don't have to be defensive here).

Cool. Done.

> [2]
> + @return: A C{Deferred} that will fire with a turth value for the
>
> typo: truth (in 2 places).

Yikes. There were a bunch more, too. Hopefully I got them all ;-)

> [3] Nice factoring with _parse_truth_return. Can you use it in the other
> places where similar thing is done?

Thanks! Done.

> +1!

Thanks!

d

[lp:~oubiwann/txaws/432540-more-securitygroup-methods]

32. By Duncan McGreggor on 2009-09-25

- Changed other functions that had custom parsing for truth returns to use the
  newly refactored method (therve 1).
- Removed debug print from unit test.

33. By Duncan McGreggor on 2009-09-25

Fixed typos (therve 2).

34. By Duncan McGreggor on 2009-09-25

Fixed a bunch more typos.

35. By Duncan McGreggor on 2009-09-25

Merged from trunk.

=== modified file 'txaws/ec2/client.py'
--- txaws/ec2/client.py	2009-09-18 14:51:21 +0000
+++ txaws/ec2/client.py	2009-09-25 05:33:10 +0000
@@ -57,9 +57,9 @@
         """Describe current instances."""
         q = self.query_factory("DescribeInstances", self.creds, self.endpoint)
         d = q.submit()
-        return d.addCallback(self._parse_instances)
+        return d.addCallback(self._parse_describe_instances)
 
-    def _parse_instances(self, xml_bytes):
+    def _parse_describe_instances(self, xml_bytes):
         """
         Parse the reservations XML payload that is returned from an AWS
         describeInstances API call.
@@ -165,9 +165,9 @@
         query = self.query_factory("DescribeSecurityGroups", self.creds,
                                    self.endpoint, group_names)
         d = query.submit()
-        return d.addCallback(self._parse_security_groups)
+        return d.addCallback(self._parse_describe_security_groups)
 
-    def _parse_security_groups(self, xml_bytes):
+    def _parse_describe_security_groups(self, xml_bytes):
         """Parse the XML returned by the C{DescribeSecurityGroups} function.
 
         @param xml_bytes: XML bytes with a C{DescribeSecurityGroupsResponse}
@@ -207,6 +207,205 @@
             result.append(security_group)
         return result
 
+    def create_security_group(self, name, description):
+        """Create security group.
+
+        @param name: Name of the new security group.
+        @param description: Description of the new security group.
+        @return: A C{Deferred} that will fire with a truth value for the
+            success of the operation.
+        """
+        group_names = None
+        parameters = {"GroupName":  name, "GroupDescription": description}
+        query = self.query_factory("CreateSecurityGroup", self.creds,
+                                   self.endpoint, parameters)
+        d = query.submit()
+        return d.addCallback(self._parse_truth_return)
+
+    def _parse_truth_return(self, xml_bytes):
+        root = XML(xml_bytes)
+        return root.findtext("return") == "true"
+
+    def delete_security_group(self, name):
+        """
+        @param name: Name of the new security group.
+        @return: A C{Deferred} that will fire with a truth value for the
+            success of the operation.
+        """
+        parameter = {"GroupName":  name}
+        query = self.query_factory("DeleteSecurityGroup", self.creds,
+                                   self.endpoint, parameter)
+        d = query.submit()
+        return d.addCallback(self._parse_truth_return)
+
+    def authorize_security_group(
+        self, group_name, source_group_name="", source_group_owner_id="",
+        ip_protocol="", from_port="", to_port="", cidr_ip=""):
+        """
+        There are two ways to use C{authorize_security_group}:
+            1) associate an existing group (source group) with the one that you
+            are targeting (group_name) with an authorization update; or
+            2) associate a set of IP permissions with the group you are
+            targeting with an authorization update.
+
+        @param group_name: The group you will be modifying with a new
+            authorization.
+
+        Optionally, the following parameters:
+        @param source_group_name: Name of security group to authorize access to
+            when operating on a user/group pair.
+        @param source_group_owner_id: Owner of security group to authorize
+            access to when operating on a user/group pair.
+
+        If those parameters are not specified, then the following must be:
+        @param ip_protocol: IP protocol to authorize access to when operating
+            on a CIDR IP.
+        @param from_port: Bottom of port range to authorize access to when
+            operating on a CIDR IP. This contains the ICMP type if ICMP is
+            being authorized.
+        @param to_port: Top of port range to authorize access to when operating
+            on a CIDR IP. This contains the ICMP code if ICMP is being
+            authorized.
+        @param cidr_ip: CIDR IP range to authorize access to when operating on
+            a CIDR IP.
+
+        @return: A C{Deferred} that will fire with a truth value for the
+            success of the operation.
+        """
+        if source_group_name and source_group_owner_id:
+            parameters = {
+                "SourceSecurityGroupName": source_group_name,
+                "SourceSecurityGroupOwnerId": source_group_owner_id,
+                }
+        elif ip_protocol and from_port and to_port and cidr_ip:
+            parameters = {
+                "IpProtocol": ip_protocol,
+                "FromPort": from_port,
+                "ToPort": to_port,
+                "CidrIp": cidr_ip,
+                }
+        else:
+            msg = ("You must specify either both group parameters or "
+                   "all the ip parameters.")
+            raise ValueError(msg)
+        parameters["GroupName"] = group_name
+        query = self.query_factory("AuthorizeSecurityGroupIngress", self.creds,
+                                   self.endpoint, parameters)
+        d = query.submit()
+        return d.addCallback(self._parse_truth_return)
+
+    def authorize_group_permission(
+        self, group_name, source_group_name, source_group_owner_id):
+        """
+        This is a convenience function that wraps the "authorize group"
+        functionality of the C{authorize_security_group} method.
+
+        For an explanation of the parameters, see C{authorize_security_group}.
+        """
+        d = self.authorize_security_group(
+            group_name,
+            source_group_name=source_group_name,
+            source_group_owner_id=source_group_owner_id)
+        return d
+
+    def authorize_ip_permission(
+        self, group_name, ip_protocol, from_port, to_port, cidr_ip):
+        """
+        This is a convenience function that wraps the "authorize ip
+        permission" functionality of the C{authorize_security_group} method.
+
+        For an explanation of the parameters, see C{authorize_security_group}.
+        """
+        d = self.authorize_security_group(
+            group_name,
+            ip_protocol=ip_protocol, from_port=from_port, to_port=to_port,
+            cidr_ip=cidr_ip)
+        return d
+
+    def revoke_security_group(
+        self, group_name, source_group_name="", source_group_owner_id="",
+        ip_protocol="", from_port="", to_port="", cidr_ip=""):
+        """
+        There are two ways to use C{revoke_security_group}:
+            1) associate an existing group (source group) with the one that you
+            are targeting (group_name) with the revoke update; or
+            2) associate a set of IP permissions with the group you are
+            targeting with a revoke update.
+
+        @param group_name: The group you will be modifying with an
+            authorization removal.
+
+        Optionally, the following parameters:
+        @param source_group_name: Name of security group to revoke access from
+            when operating on a user/group pair.
+        @param source_group_owner_id: Owner of security group to revoke
+            access from when operating on a user/group pair.
+
+        If those parameters are not specified, then the following must be:
+        @param ip_protocol: IP protocol to revoke access from when operating
+            on a CIDR IP.
+        @param from_port: Bottom of port range to revoke access from when
+            operating on a CIDR IP. This contains the ICMP type if ICMP is
+            being revoked.
+        @param to_port: Top of port range to revoke access from when operating
+            on a CIDR IP. This contains the ICMP code if ICMP is being
+            revoked.
+        @param cidr_ip: CIDR IP range to revoke access from when operating on
+            a CIDR IP.
+
+        @return: A C{Deferred} that will fire with a truth value for the
+            success of the operation.
+        """
+        if source_group_name and source_group_owner_id:
+            parameters = {
+                "SourceSecurityGroupName": source_group_name,
+                "SourceSecurityGroupOwnerId": source_group_owner_id,
+                }
+        elif ip_protocol and from_port and to_port and cidr_ip:
+            parameters = {
+                "IpProtocol": ip_protocol,
+                "FromPort": from_port,
+                "ToPort": to_port,
+                "CidrIp": cidr_ip,
+                }
+        else:
+            msg = ("You must specify either both group parameters or "
+                   "all the ip parameters.")
+            raise ValueError(msg)
+        parameters["GroupName"] = group_name
+        query = self.query_factory("RevokeSecurityGroupIngress", self.creds,
+                                   self.endpoint, parameters)
+        d = query.submit()
+        return d.addCallback(self._parse_truth_return)
+
+    def revoke_group_permission(
+        self, group_name, source_group_name, source_group_owner_id):
+        """
+        This is a convenience function that wraps the "authorize group"
+        functionality of the C{authorize_security_group} method.
+
+        For an explanation of the parameters, see C{authorize_security_group}.
+        """
+        d = self.revoke_security_group(
+            group_name,
+            source_group_name=source_group_name,
+            source_group_owner_id=source_group_owner_id)
+        return d
+
+    def revoke_ip_permission(
+        self, group_name, ip_protocol, from_port, to_port, cidr_ip):
+        """
+        This is a convenience function that wraps the "authorize ip
+        permission" functionality of the C{authorize_security_group} method.
+
+        For an explanation of the parameters, see C{authorize_security_group}.
+        """
+        d = self.revoke_security_group(
+            group_name,
+            ip_protocol=ip_protocol, from_port=from_port, to_port=to_port,
+            cidr_ip=cidr_ip)
+        return d
+
     def describe_volumes(self, *volume_ids):
         """Describe available volumes."""
         volumeset = {}
@@ -215,9 +414,9 @@
         q = self.query_factory(
             "DescribeVolumes", self.creds, self.endpoint, volumeset)
         d = q.submit()
-        return d.addCallback(self._parse_volumes)
+        return d.addCallback(self._parse_describe_volumes)
 
-    def _parse_volumes(self, xml_bytes):
+    def _parse_describe_volumes(self, xml_bytes):
         root = XML(xml_bytes)
         result = []
         for volume_data in root.find("volumeSet"):
@@ -274,11 +473,7 @@
         q = self.query_factory(
             "DeleteVolume", self.creds, self.endpoint, {"VolumeId": volume_id})
         d = q.submit()
-        return d.addCallback(self._parse_delete_volume)
-
-    def _parse_delete_volume(self, xml_bytes):
-        root = XML(xml_bytes)
-        return root.findtext("return") == "true"
+        return d.addCallback(self._parse_truth_return)
 
     def describe_snapshots(self, *snapshot_ids):
         """Describe available snapshots."""
@@ -334,11 +529,7 @@
             "DeleteSnapshot", self.creds, self.endpoint,
             {"SnapshotId": snapshot_id})
         d = q.submit()
-        return d.addCallback(self._parse_delete_snapshot)
-
-    def _parse_delete_snapshot(self, xml_bytes):
-        root = XML(xml_bytes)
-        return root.findtext("return") == "true"
+        return d.addCallback(self._parse_truth_return)
 
     def attach_volume(self, volume_id, instance_id, device):
         """Attach the given volume to the specified instance at C{device}."""
@@ -401,19 +592,7 @@
             "DeleteKeyPair", self.creds, self.endpoint,
             {"KeyName": keypair_name})
         d = q.submit()
-        return d.addCallback(self._parse_delete_keypair)
-
-    def _parse_delete_keypair(self, xml_bytes):
-        results = []
-        keypair_data = XML(xml_bytes)
-        result = keypair_data.findtext("return")
-        if not result:
-            result = False
-        elif result.lower() == "true":
-            result = True
-        else:
-            result = False
-        return result
+        return d.addCallback(self._parse_truth_return)
 
 
 class Query(object):
@@ -471,7 +650,7 @@
         self.params["Signature"] = self.creds.sign(self.signing_text())
 
     def sorted_params(self):
-        """Return the query params sorted appropriately for signing."""
+        """Return the query parameters sorted appropriately for signing."""
         return sorted(self.params.items())
 
     def get_page(self, url, *args, **kwds):

=== modified file 'txaws/ec2/tests/test_client.py'
--- txaws/ec2/tests/test_client.py	2009-09-15 21:12:04 +0000
+++ txaws/ec2/tests/test_client.py	2009-09-25 05:33:10 +0000
@@ -7,7 +7,7 @@
 import os
 
 from twisted.internet import reactor
-from twisted.internet.defer import succeed
+from twisted.internet.defer import succeed, fail
 from twisted.python.failure import Failure
 from twisted.python.filepath import FilePath
 from twisted.web import server, static, util
@@ -71,6 +71,9 @@
         ec2 = client.EC2Client(creds=creds)
         self.assertEqual(creds, ec2.creds)
 
+
+class EC2ClientInstancesTestCase(TXAWSTestCase):
+
     def check_parsed_instances(self, results):
         instance = results[0]
         # check reservations
@@ -130,7 +133,7 @@
     def test_parse_reservation(self):
         creds = AWSCredentials("foo", "bar")
         ec2 = client.EC2Client(creds=creds)
-        results = ec2._parse_instances(
+        results = ec2._parse_describe_instances(
             payload.sample_describe_instances_result)
         self.check_parsed_instances(results)
 
@@ -185,6 +188,9 @@
         d.addCallback(check_transition)
         return d
 
+
+class EC2ClientSecurityGroupsTestCase(TXAWSTestCase):
+
     def test_describe_security_groups(self):
         """
         L{EC2Client.describe_security_groups} returns a C{Deferred} that
@@ -200,7 +206,7 @@
             def submit(self):
                 return succeed(payload.sample_describe_security_groups_result)
 
-        def assert_security_groups(security_groups):
+        def check_results(security_groups):
             [security_group] = security_groups
             self.assertEquals(security_group.owner_id,
                               "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM")
@@ -214,9 +220,8 @@
 
         creds = AWSCredentials("foo", "bar")
         ec2 = client.EC2Client(creds, query_factory=StubQuery)
-        security_groups = ec2.describe_security_groups()
-        security_groups.addCallback(assert_security_groups)
-        return security_groups
+        d = ec2.describe_security_groups()
+        return d.addCallback(check_results)
 
     def test_describe_security_groups_with_multiple_results(self):
         """
@@ -234,7 +239,7 @@
                 return succeed(
                     payload.sample_describe_security_groups_multiple_result)
 
-        def assert_security_groups(security_groups):
+        def check_results(security_groups):
             self.assertEquals(len(security_groups), 2)
 
             security_group = security_groups[0]
@@ -263,9 +268,8 @@
 
         creds = AWSCredentials("foo", "bar")
         ec2 = client.EC2Client(creds, query_factory=StubQuery)
-        security_groups = ec2.describe_security_groups()
-        security_groups.addCallback(assert_security_groups)
-        return security_groups
+        d = ec2.describe_security_groups()
+        return d.addCallback(check_results)
 
     def test_describe_security_groups_with_name(self):
         """
@@ -281,267 +285,354 @@
             def submit(self):
                 return succeed(payload.sample_describe_security_groups_result)
 
-        def assert_security_groups(security_groups):
+        def check_result(security_groups):
             [security_group] = security_groups
             self.assertEquals(security_group.name, "WebServers")
 
         creds = AWSCredentials("foo", "bar")
         ec2 = client.EC2Client(creds, query_factory=StubQuery)
-        security_groups = ec2.describe_security_groups("WebServers")
-        security_groups.addCallback(assert_security_groups)
-        return security_groups
-
-
-class QueryTestCase(TXAWSTestCase):
-
-    def setUp(self):
-        TXAWSTestCase.setUp(self)
-        self.creds = AWSCredentials("foo", "bar")
-        self.endpoint = AWSServiceEndpoint(uri=EC2_ENDPOINT_US)
-
-    def test_init_minimum(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint)
-        self.assertTrue("Timestamp" in query.params)
-        del query.params["Timestamp"]
-        self.assertEqual(
-            {"AWSAccessKeyId": "foo",
-             "Action": "DescribeInstances",
-             "SignatureMethod": "HmacSHA1",
-             "SignatureVersion": "2",
-             "Version": "2008-12-01"},
-            query.params)
-
-    def test_init_requires_action(self):
-        self.assertRaises(TypeError, client.Query)
-
-    def test_init_requires_creds(self):
-        self.assertRaises(TypeError, client.Query, None)
-
-    def test_init_other_args_are_params(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint,
-            {"InstanceId.0": "12345"},
-            time_tuple=(2007,11,12,13,14,15,0,0,0))
-        self.assertEqual(
-            {"AWSAccessKeyId": "foo",
-             "Action": "DescribeInstances",
-             "InstanceId.0": "12345",
-             "SignatureMethod": "HmacSHA1",
-             "SignatureVersion": "2",
-             "Timestamp": "2007-11-12T13:14:15Z",
-             "Version": "2008-12-01"},
-            query.params)
-
-    def test_sorted_params(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint,
-            {"fun": "games"},
-            time_tuple=(2007,11,12,13,14,15,0,0,0))
-        self.assertEqual([
-            ("AWSAccessKeyId", "foo"),
-            ("Action", "DescribeInstances"),
-            ("SignatureMethod", "HmacSHA1"),
-            ("SignatureVersion", "2"),
-            ("Timestamp", "2007-11-12T13:14:15Z"),
-            ("Version", "2008-12-01"),
-            ("fun", "games"),
-            ], query.sorted_params())
-
-    def test_encode_unreserved(self):
-        all_unreserved = ("ABCDEFGHIJKLMNOPQRSTUVWXYZ"
-            "abcdefghijklmnopqrstuvwxyz0123456789-_.~")
-        query = client.Query("DescribeInstances", self.creds, self.endpoint)
-        self.assertEqual(all_unreserved, query.encode(all_unreserved))
-
-    def test_encode_space(self):
-        """This may be just "url encode", but the AWS manual isn't clear."""
-        query = client.Query("DescribeInstances", self.creds, self.endpoint)
-        self.assertEqual("a%20space", query.encode("a space"))
-
-    def test_canonical_query(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint,
-            {"fu n": "g/ames", "argwithnovalue":"",
-             "InstanceId.1": "i-1234"},
-            time_tuple=(2007,11,12,13,14,15,0,0,0))
-        expected_query = ("AWSAccessKeyId=foo&Action=DescribeInstances"
-            "&InstanceId.1=i-1234"
-            "&SignatureMethod=HmacSHA1&SignatureVersion=2&"
-            "Timestamp=2007-11-12T13%3A14%3A15Z&Version=2008-12-01&"
-            "argwithnovalue=&fu%20n=g%2Fames")
-        self.assertEqual(expected_query, query.canonical_query_params())
-
-    def test_signing_text(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint,
-            time_tuple=(2007,11,12,13,14,15,0,0,0))
-        signing_text = ("GET\n%s\n/\n" % self.endpoint.host +
-            "AWSAccessKeyId=foo&Action=DescribeInstances&"
-            "SignatureMethod=HmacSHA1&SignatureVersion=2&"
-            "Timestamp=2007-11-12T13%3A14%3A15Z&Version=2008-12-01")
-        self.assertEqual(signing_text, query.signing_text())
-
-    def test_sign(self):
-        query = client.Query("DescribeInstances", self.creds, self.endpoint,
-            time_tuple=(2007,11,12,13,14,15,0,0,0))
-        query.sign()
-        self.assertEqual("JuCpwFA2H4OVF3Ql/lAQs+V6iMc=",
-            query.params["Signature"])
-
-    def test_submit_400(self):
-        """A 4xx response status from EC2 should raise a txAWS EC2Error."""
-        status = 400
-        self.addCleanup(setattr, client.Query, "get_page",
-                        client.Query.get_page)
-        fake_page_getter = FakePageGetter(
-            status, payload.sample_ec2_error_message)
-        client.Query.get_page = fake_page_getter.get_page_with_exception
+        d = ec2.describe_security_groups("WebServers")
+        return d.addCallback(check_result)
+
+    def test_create_security_group(self):
+        """
+        L{EC2Client.create_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "CreateSecurityGroup")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "GroupDescription": "The group for the web server farm.",
+                    })
+            def submit(self):
+                return succeed(payload.sample_create_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.create_security_group(
+            "WebServers",
+            "The group for the web server farm.")
+        return self.assertTrue(d)
+
+    def test_delete_security_group(self):
+        """
+        L{EC2Client.delete_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "DeleteSecurityGroup")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    })
+            def submit(self):
+                return succeed(payload.sample_delete_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.delete_security_group("WebServers")
+        return self.assertTrue(d)
+
+    def test_delete_security_group_failure(self):
+        """
+        L{EC2Client.delete_security_group} returns a C{Deferred} that
+        eventually fires with a failure when EC2 is asked to delete a group
+        that another group uses in that other group's policy.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "DeleteSecurityGroup")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "GroupReferredTo",
+                    })
+            def submit(self):
+                error = EC2Error(payload.sample_delete_security_group_failure)
+                return fail(error)
 
         def check_error(error):
-            self.assertTrue(isinstance(error, EC2Error))
-            self.assertEquals(error.get_error_codes(), "Error.Code")
             self.assertEquals(
-                error.get_error_messages(),
-                "Message for Error.Code")
-            self.assertEquals(error.status, status)
-            self.assertEquals(error.response, payload.sample_ec2_error_message)
-        
-        query = client.Query(
-            'BadQuery', self.creds, self.endpoint,
-            time_tuple=(2009,8,15,13,14,15,0,0,0))
+                str(error),
+                ("Error Message: Group groupID1:GroupReferredTo is used by "
+                 "groups: groupID2:UsingGroup"))
 
-        failure = query.submit()
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        failure = ec2.delete_security_group("GroupReferredTo")
         d = self.assertFailure(failure, EC2Error)
-        d.addCallback(check_error)
-        return d
-
-    def test_submit_500(self):
-        """
-        A 5xx response status from EC2 should raise the original Twisted
-        exception.
-        """
-        status = 500
-        self.addCleanup(setattr, client.Query, "get_page",
-                        client.Query.get_page)
-        fake_page_getter = FakePageGetter(
-            status, payload.sample_ec2_error_message)
-        client.Query.get_page = fake_page_getter.get_page_with_exception
-
-        def check_error(error):
-            self.assertFalse(isinstance(error, EC2Error))
-            self.assertEquals(error.status, status)
-            self.assertEquals(str(error), "500 There's been an error")
-        
-        query = client.Query(
-            'BadQuery', self.creds, self.endpoint,
-            time_tuple=(2009,8,15,13,14,15,0,0,0))
-
-        failure = query.submit()
-        d = self.assertFailure(failure, Error)
-        d.addCallback(check_error)
-        return d
-
-
-class QueryPageGetterTestCase(TXAWSTestCase):
-
-    def setUp(self):
-        TXAWSTestCase.setUp(self)
-        self.creds = AWSCredentials("foo", "bar")
-        self.endpoint = AWSServiceEndpoint(uri=EC2_ENDPOINT_US)
-        self.twisted_client_test_setup()
-        self.cleanupServerConnections = 0
-
-    def tearDown(self):
-        """Copied from twisted.web.test.test_webclient."""
-        # If the test indicated it might leave some server-side connections
-        # around, clean them up.
-        connections = self.wrapper.protocols.keys()
-        # If there are fewer server-side connections than requested,
-        # that's okay.  Some might have noticed that the client closed
-        # the connection and cleaned up after themselves.
-        for n in range(min(len(connections), self.cleanupServerConnections)):
-            proto = connections.pop()
-            #msg("Closing %r" % (proto,))
-            proto.transport.loseConnection()
-        if connections:
-            #msg("Some left-over connections; this test is probably buggy.")
-            pass
-        return self.port.stopListening()
-
-    def _listen(self, site):
-        return reactor.listenTCP(0, site, interface="127.0.0.1")
-
-    def twisted_client_test_setup(self):
-        name = self.mktemp()
-        os.mkdir(name)
-        FilePath(name).child("file").setContent("0123456789")
-        resource = static.File(name)
-        resource.putChild("redirect", util.Redirect("/file"))
-        self.site = server.Site(resource, timeout=None)
-        self.wrapper = WrappingFactory(self.site)
-        self.port = self._listen(self.wrapper)
-        self.portno = self.port.getHost().port
-
-    def get_url(self, path):
-        return "http://127.0.0.1:%d/%s" % (self.portno, path)
-
-    def test_get_page(self):
-        """Copied from twisted.web.test.test_webclient."""
-        query = client.Query(
-            'DummyQuery', self.creds, self.endpoint,
-            time_tuple=(2009,8,17,13,14,15,0,0,0))
-        deferred = query.get_page(self.get_url("file"))
-        deferred.addCallback(self.assertEquals, "0123456789")
-        return deferred
-
-
-class EC2ErrorWrapperTestCase(TXAWSTestCase):
-
-    def setUp(self):
-        TXAWSTestCase.setUp(self)
-
-    def get_failure(self, status=None, type=None, message=""):
-        failure = Failure(type(message))
-        failure.value.response = payload.sample_ec2_error_message
-        failure.value.status = status
-        return failure
-
-    def test_302_error(self):
-        failure = self.get_failure(302, Exception, "found")
-        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
-        self.assertEquals(failure.type, type(error))
-        self.assertFalse(isinstance(error, EC2Error))
-        self.assertTrue(isinstance(error, Exception))
-        self.assertEquals(error.message, "found")
-
-    def test_400_error(self):
-        failure = self.get_failure(400, Exception)
-        error = self.assertRaises(EC2Error, client.ec2_error_wrapper, failure)
-        self.assertNotEquals(failure.type, type(error))
-        self.assertTrue(isinstance(error, EC2Error))
-        self.assertEquals(error.get_error_codes(), "Error.Code")
-        self.assertEquals(error.get_error_messages(), "Message for Error.Code")
-
-    def test_404_error(self):
-        failure = self.get_failure(404, Exception)
-        error = self.assertRaises(EC2Error, client.ec2_error_wrapper, failure)
-        self.assertNotEquals(failure.type, type(error))
-        self.assertTrue(isinstance(error, EC2Error))
-        self.assertEquals(error.get_error_codes(), "Error.Code")
-        self.assertEquals(error.get_error_messages(), "Message for Error.Code")
-
-    def test_500_error(self):
-        failure = self.get_failure(500, Exception, "A server error occurred")
-        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
-        self.assertFalse(isinstance(error, EC2Error))
-        self.assertTrue(isinstance(error, Exception))
-        self.assertEquals(error.message, "A server error occurred")
-
-    def test_timeout_error(self):
-        failure = self.get_failure(type=Exception, message="timeout")
-        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
-        self.assertFalse(isinstance(error, EC2Error))
-        self.assertTrue(isinstance(error, Exception))
-        self.assertEquals(error.message, "timeout")
-
-
-class EBSTestCase(TXAWSTestCase):
+        return d.addCallback(check_error)
+
+    def test_authorize_security_group_with_user_group_pair(self):
+        """
+        L{EC2Client.authorize_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions; this test checks the first
+        way.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "AuthorizeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "SourceSecurityGroupName": "AppServers",
+                    "SourceSecurityGroupOwnerId": "123456789123",
+                    })
+            def submit(self):
+                return succeed(payload.sample_authorize_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.authorize_security_group(
+            "WebServers", source_group_name="AppServers",
+            source_group_owner_id="123456789123")
+        return self.assertTrue(d)
+
+    def test_authorize_security_group_with_ip_permissions(self):
+        """
+        L{EC2Client.authorize_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions; this test checks the second
+        way.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "AuthorizeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "FromPort": "22", "ToPort": "80",
+                    "IpProtocol": "tcp", "CidrIp": "0.0.0.0/0",
+                    })
+            def submit(self):
+                return succeed(payload.sample_authorize_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.authorize_security_group(
+            "WebServers", ip_protocol="tcp", from_port="22", to_port="80",
+            cidr_ip="0.0.0.0/0")
+        return self.assertTrue(d)
+
+    def test_authorize_security_group_with_missing_parameters(self):
+        """
+        L{EC2Client.authorize_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions. If not all group-setting
+        parameters are set and not all IP permission parameters are set, an
+        error is raised.
+        """
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds)
+        self.assertRaises(ValueError, ec2.authorize_security_group,
+                "WebServers", ip_protocol="tcp", from_port="22")
+        try:
+            d = ec2.authorize_security_group(
+                "WebServers", ip_protocol="tcp", from_port="22")
+        except Exception, error:
+            self.assertEquals(
+                str(error), 
+                ("You must specify either both group parameters or all the "
+                 "ip parameters."))
+
+    def test_authorize_group_permission(self):
+        """
+        L{EC2Client.authorize_group_permission} returns a C{Deferred}
+        that eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "AuthorizeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "SourceSecurityGroupName": "AppServers",
+                    "SourceSecurityGroupOwnerId": "123456789123",
+                    })
+            def submit(self):
+                return succeed(payload.sample_authorize_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.authorize_group_permission(
+            "WebServers", source_group_name="AppServers",
+            source_group_owner_id="123456789123")
+        return self.assertTrue(d)
+
+    def test_authorize_ip_permission(self):
+        """
+        L{EC2Client.authorize_ip_permission} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "AuthorizeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "FromPort": "22", "ToPort": "80",
+                    "IpProtocol": "tcp", "CidrIp": "0.0.0.0/0",
+                    })
+            def submit(self):
+                return succeed(payload.sample_authorize_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.authorize_ip_permission(
+            "WebServers", ip_protocol="tcp", from_port="22", to_port="80",
+            cidr_ip="0.0.0.0/0")
+        return self.assertTrue(d)
+
+    def test_revoke_security_group_with_user_group_pair(self):
+        """
+        L{EC2Client.revoke_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions; this test checks the first
+        way.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "RevokeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "SourceSecurityGroupName": "AppServers",
+                    "SourceSecurityGroupOwnerId": "123456789123",
+                    })
+            def submit(self):
+                return succeed(payload.sample_revoke_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.revoke_security_group(
+            "WebServers", source_group_name="AppServers",
+            source_group_owner_id="123456789123")
+        return self.assertTrue(d)
+
+    def test_revoke_security_group_with_ip_permissions(self):
+        """
+        L{EC2Client.revoke_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions; this test checks the second
+        way.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "RevokeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "FromPort": "22", "ToPort": "80",
+                    "IpProtocol": "tcp", "CidrIp": "0.0.0.0/0",
+                    })
+            def submit(self):
+                return succeed(payload.sample_revoke_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.revoke_security_group(
+            "WebServers", ip_protocol="tcp", from_port="22", to_port="80",
+            cidr_ip="0.0.0.0/0")
+        return self.assertTrue(d)
+
+    def test_revoke_security_group_with_missing_parameters(self):
+        """
+        L{EC2Client.revoke_security_group} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation. There are two ways to use the method: set another group's
+        IP permissions or set new IP permissions. If not all group-setting
+        parameters are set and not all IP permission parameters are set, an
+        error is raised.
+        """
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds)
+        self.assertRaises(ValueError, ec2.authorize_security_group,
+                "WebServers", ip_protocol="tcp", from_port="22")
+        try:
+            d = ec2.authorize_security_group(
+                "WebServers", ip_protocol="tcp", from_port="22")
+        except Exception, error:
+            self.assertEquals(
+                str(error), 
+                ("You must specify either both group parameters or all the "
+                 "ip parameters."))
+
+    def test_revoke_group_permission(self):
+        """
+        L{EC2Client.revoke_group_permission} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "RevokeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "SourceSecurityGroupName": "AppServers",
+                    "SourceSecurityGroupOwnerId": "123456789123",
+                    })
+            def submit(self):
+                return succeed(payload.sample_revoke_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.revoke_group_permission(
+            "WebServers", source_group_name="AppServers",
+            source_group_owner_id="123456789123")
+        return self.assertTrue(d)
+
+    def test_revoke_ip_permission(self):
+        """
+        L{EC2Client.revoke_ip_permission} returns a C{Deferred} that
+        eventually fires with a true value, indicating the success of the
+        operation.
+        """
+        class StubQuery(object):
+            def __init__(stub, action, creds, endpoint, other_params=None):
+                self.assertEqual(action, "RevokeSecurityGroupIngress")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupName": "WebServers",
+                    "FromPort": "22", "ToPort": "80",
+                    "IpProtocol": "tcp", "CidrIp": "0.0.0.0/0",
+                    })
+            def submit(self):
+                return succeed(payload.sample_revoke_security_group)
+
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds, query_factory=StubQuery)
+        d = ec2.revoke_ip_permission(
+            "WebServers", ip_protocol="tcp", from_port="22", to_port="80",
+            cidr_ip="0.0.0.0/0")
+        return self.assertTrue(d)
+
+
+class EC2ClientEBSTestCase(TXAWSTestCase):
 
     def setUp(self):
         TXAWSTestCase.setUp(self)
@@ -970,3 +1061,252 @@
         d = ec2.delete_keypair("example-key-name")
         d.addCallback(self.assertFalse)
         return d
+
+
+class EC2ErrorWrapperTestCase(TXAWSTestCase):
+
+    def setUp(self):
+        TXAWSTestCase.setUp(self)
+
+    def get_failure(self, status=None, type=None, message=""):
+        failure = Failure(type(message))
+        failure.value.response = payload.sample_ec2_error_message
+        failure.value.status = status
+        return failure
+
+    def test_302_error(self):
+        failure = self.get_failure(302, Exception, "found")
+        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
+        self.assertEquals(failure.type, type(error))
+        self.assertFalse(isinstance(error, EC2Error))
+        self.assertTrue(isinstance(error, Exception))
+        self.assertEquals(error.message, "found")
+
+    def test_400_error(self):
+        failure = self.get_failure(400, Exception)
+        error = self.assertRaises(EC2Error, client.ec2_error_wrapper, failure)
+        self.assertNotEquals(failure.type, type(error))
+        self.assertTrue(isinstance(error, EC2Error))
+        self.assertEquals(error.get_error_codes(), "Error.Code")
+        self.assertEquals(error.get_error_messages(), "Message for Error.Code")
+
+    def test_404_error(self):
+        failure = self.get_failure(404, Exception)
+        error = self.assertRaises(EC2Error, client.ec2_error_wrapper, failure)
+        self.assertNotEquals(failure.type, type(error))
+        self.assertTrue(isinstance(error, EC2Error))
+        self.assertEquals(error.get_error_codes(), "Error.Code")
+        self.assertEquals(error.get_error_messages(), "Message for Error.Code")
+
+    def test_500_error(self):
+        failure = self.get_failure(500, Exception, "A server error occurred")
+        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
+        self.assertFalse(isinstance(error, EC2Error))
+        self.assertTrue(isinstance(error, Exception))
+        self.assertEquals(error.message, "A server error occurred")
+
+    def test_timeout_error(self):
+        failure = self.get_failure(type=Exception, message="timeout")
+        error = self.assertRaises(Exception, client.ec2_error_wrapper, failure)
+        self.assertFalse(isinstance(error, EC2Error))
+        self.assertTrue(isinstance(error, Exception))
+        self.assertEquals(error.message, "timeout")
+
+
+class QueryTestCase(TXAWSTestCase):
+
+    def setUp(self):
+        TXAWSTestCase.setUp(self)
+        self.creds = AWSCredentials("foo", "bar")
+        self.endpoint = AWSServiceEndpoint(uri=EC2_ENDPOINT_US)
+
+    def test_init_minimum(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint)
+        self.assertTrue("Timestamp" in query.params)
+        del query.params["Timestamp"]
+        self.assertEqual(
+            {"AWSAccessKeyId": "foo",
+             "Action": "DescribeInstances",
+             "SignatureMethod": "HmacSHA1",
+             "SignatureVersion": "2",
+             "Version": "2008-12-01"},
+            query.params)
+
+    def test_init_requires_action(self):
+        self.assertRaises(TypeError, client.Query)
+
+    def test_init_requires_creds(self):
+        self.assertRaises(TypeError, client.Query, None)
+
+    def test_init_other_args_are_params(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint,
+            {"InstanceId.0": "12345"},
+            time_tuple=(2007,11,12,13,14,15,0,0,0))
+        self.assertEqual(
+            {"AWSAccessKeyId": "foo",
+             "Action": "DescribeInstances",
+             "InstanceId.0": "12345",
+             "SignatureMethod": "HmacSHA1",
+             "SignatureVersion": "2",
+             "Timestamp": "2007-11-12T13:14:15Z",
+             "Version": "2008-12-01"},
+            query.params)
+
+    def test_sorted_params(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint,
+            {"fun": "games"},
+            time_tuple=(2007,11,12,13,14,15,0,0,0))
+        self.assertEqual([
+            ("AWSAccessKeyId", "foo"),
+            ("Action", "DescribeInstances"),
+            ("SignatureMethod", "HmacSHA1"),
+            ("SignatureVersion", "2"),
+            ("Timestamp", "2007-11-12T13:14:15Z"),
+            ("Version", "2008-12-01"),
+            ("fun", "games"),
+            ], query.sorted_params())
+
+    def test_encode_unreserved(self):
+        all_unreserved = ("ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+            "abcdefghijklmnopqrstuvwxyz0123456789-_.~")
+        query = client.Query("DescribeInstances", self.creds, self.endpoint)
+        self.assertEqual(all_unreserved, query.encode(all_unreserved))
+
+    def test_encode_space(self):
+        """This may be just "url encode", but the AWS manual isn't clear."""
+        query = client.Query("DescribeInstances", self.creds, self.endpoint)
+        self.assertEqual("a%20space", query.encode("a space"))
+
+    def test_canonical_query(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint,
+            {"fu n": "g/ames", "argwithnovalue":"",
+             "InstanceId.1": "i-1234"},
+            time_tuple=(2007,11,12,13,14,15,0,0,0))
+        expected_query = ("AWSAccessKeyId=foo&Action=DescribeInstances"
+            "&InstanceId.1=i-1234"
+            "&SignatureMethod=HmacSHA1&SignatureVersion=2&"
+            "Timestamp=2007-11-12T13%3A14%3A15Z&Version=2008-12-01&"
+            "argwithnovalue=&fu%20n=g%2Fames")
+        self.assertEqual(expected_query, query.canonical_query_params())
+
+    def test_signing_text(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint,
+            time_tuple=(2007,11,12,13,14,15,0,0,0))
+        signing_text = ("GET\n%s\n/\n" % self.endpoint.host +
+            "AWSAccessKeyId=foo&Action=DescribeInstances&"
+            "SignatureMethod=HmacSHA1&SignatureVersion=2&"
+            "Timestamp=2007-11-12T13%3A14%3A15Z&Version=2008-12-01")
+        self.assertEqual(signing_text, query.signing_text())
+
+    def test_sign(self):
+        query = client.Query("DescribeInstances", self.creds, self.endpoint,
+            time_tuple=(2007,11,12,13,14,15,0,0,0))
+        query.sign()
+        self.assertEqual("JuCpwFA2H4OVF3Ql/lAQs+V6iMc=",
+            query.params["Signature"])
+
+    def test_submit_400(self):
+        """A 4xx response status from EC2 should raise a txAWS EC2Error."""
+        status = 400
+        self.addCleanup(setattr, client.Query, "get_page",
+                        client.Query.get_page)
+        fake_page_getter = FakePageGetter(
+            status, payload.sample_ec2_error_message)
+        client.Query.get_page = fake_page_getter.get_page_with_exception
+
+        def check_error(error):
+            self.assertTrue(isinstance(error, EC2Error))
+            self.assertEquals(error.get_error_codes(), "Error.Code")
+            self.assertEquals(
+                error.get_error_messages(),
+                "Message for Error.Code")
+            self.assertEquals(error.status, status)
+            self.assertEquals(error.response, payload.sample_ec2_error_message)
+        
+        query = client.Query(
+            'BadQuery', self.creds, self.endpoint,
+            time_tuple=(2009,8,15,13,14,15,0,0,0))
+
+        failure = query.submit()
+        d = self.assertFailure(failure, EC2Error)
+        d.addCallback(check_error)
+        return d
+
+    def test_submit_500(self):
+        """
+        A 5xx response status from EC2 should raise the original Twisted
+        exception.
+        """
+        status = 500
+        self.addCleanup(setattr, client.Query, "get_page",
+                        client.Query.get_page)
+        fake_page_getter = FakePageGetter(
+            status, payload.sample_ec2_error_message)
+        client.Query.get_page = fake_page_getter.get_page_with_exception
+
+        def check_error(error):
+            self.assertFalse(isinstance(error, EC2Error))
+            self.assertEquals(error.status, status)
+            self.assertEquals(str(error), "500 There's been an error")
+        
+        query = client.Query(
+            'BadQuery', self.creds, self.endpoint,
+            time_tuple=(2009,8,15,13,14,15,0,0,0))
+
+        failure = query.submit()
+        d = self.assertFailure(failure, Error)
+        d.addCallback(check_error)
+        return d
+
+
+class QueryPageGetterTestCase(TXAWSTestCase):
+
+    def setUp(self):
+        TXAWSTestCase.setUp(self)
+        self.creds = AWSCredentials("foo", "bar")
+        self.endpoint = AWSServiceEndpoint(uri=EC2_ENDPOINT_US)
+        self.twisted_client_test_setup()
+        self.cleanupServerConnections = 0
+
+    def tearDown(self):
+        """Copied from twisted.web.test.test_webclient."""
+        # If the test indicated it might leave some server-side connections
+        # around, clean them up.
+        connections = self.wrapper.protocols.keys()
+        # If there are fewer server-side connections than requested,
+        # that's okay.  Some might have noticed that the client closed
+        # the connection and cleaned up after themselves.
+        for n in range(min(len(connections), self.cleanupServerConnections)):
+            proto = connections.pop()
+            #msg("Closing %r" % (proto,))
+            proto.transport.loseConnection()
+        if connections:
+            #msg("Some left-over connections; this test is probably buggy.")
+            pass
+        return self.port.stopListening()
+
+    def _listen(self, site):
+        return reactor.listenTCP(0, site, interface="127.0.0.1")
+
+    def twisted_client_test_setup(self):
+        name = self.mktemp()
+        os.mkdir(name)
+        FilePath(name).child("file").setContent("0123456789")
+        resource = static.File(name)
+        resource.putChild("redirect", util.Redirect("/file"))
+        self.site = server.Site(resource, timeout=None)
+        self.wrapper = WrappingFactory(self.site)
+        self.port = self._listen(self.wrapper)
+        self.portno = self.port.getHost().port
+
+    def get_url(self, path):
+        return "http://127.0.0.1:%d/%s" % (self.portno, path)
+
+    def test_get_page(self):
+        """Copied from twisted.web.test.test_webclient."""
+        query = client.Query(
+            'DummyQuery', self.creds, self.endpoint,
+            time_tuple=(2009,8,17,13,14,15,0,0,0))
+        deferred = query.get_page(self.get_url("file"))
+        deferred.addCallback(self.assertEquals, "0123456789")
+        return deferred

=== modified file 'txaws/exception.py'
--- txaws/exception.py	2009-09-10 00:32:37 +0000
+++ txaws/exception.py	2009-09-25 05:33:10 +0000
@@ -3,6 +3,7 @@
 
 from twisted.web.error import Error
 
+
 class AWSError(Error):
     """
     A base class for txAWS errors.

=== modified file 'txaws/testing/payload.py'
--- txaws/testing/payload.py	2009-09-15 22:19:14 +0000
+++ txaws/testing/payload.py	2009-09-25 05:33:10 +0000
@@ -202,6 +202,48 @@
 """ % (aws_api,)
 
 
+sample_create_security_group = """\
+<CreateSecurityGroupResponse xmlns="http://ec2.amazonaws.com/doc/%s/">
+  <return>true</return>
+</CreateSecurityGroupResponse>
+""" % (aws_api,)
+
+
+sample_delete_security_group = """\
+<DeleteSecurityGroupResponse xmlns="http://ec2.amazonaws.com/doc/%s/">
+  <return>true</return>
+</DeleteSecurityGroupResponse>
+""" % (aws_api,)
+
+
+sample_delete_security_group_failure = """\
+<?xml version="1.0"?>
+<Response>
+  <Errors>
+    <Error>
+      <Code>InvalidGroup.InUse</Code>
+      <Message>Group groupID1:GroupReferredTo is used by groups: groupID2:UsingGroup</Message>
+    </Error>
+  </Errors>
+  <RequestID>9a6df05f-9c27-47aa-81d8-6619689210cc</RequestID>
+</Response>
+"""
+
+
+sample_authorize_security_group = """\
+<AuthorizeSecurityGroupIngressResponse xmlns="http://ec2.amazonaws.com/doc/%s/">
+  <return>true</return>
+</AuthorizeSecurityGroupIngressResponse>
+""" % (aws_api,)
+
+
+sample_revoke_security_group = """\
+<RevokeSecurityGroupIngressResponse xmlns="http://ec2.amazonaws.com/doc/%s/">
+  <return>true</return>
+</RevokeSecurityGroupIngressResponse>
+""" % (aws_api,)
+
+
 sample_describe_volumes_result = """\
 <?xml version="1.0"?>
 <DescribeVolumesResponse xmlns="http://ec2.amazonaws.com/doc/%s/">