txaws-dev team mailing list archive

[Francisco Souza <f@xxxxxxxx>]

Francisco Souza has proposed merging lp:~franciscosouza/txaws/txaws-secgroupid into lp:txaws.

Requested reviews:
  txAWS Committers (txaws-dev)

For more details, see:
https://code.launchpad.net/~franciscosouza/txaws/txaws-secgroupid/+merge/133542

ec2: recognize ID on security groups

When using VPC, security groups are identified by ID, not name.

https://codereview.appspot.com/6822097/

-- 
https://code.launchpad.net/~franciscosouza/txaws/txaws-secgroupid/+merge/133542
Your team txAWS Committers is requested to review the proposed merge of lp:~franciscosouza/txaws/txaws-secgroupid into lp:txaws.

=== modified file 'txaws/ec2/client.py'
--- txaws/ec2/client.py	2012-05-05 00:17:02 +0000
+++ txaws/ec2/client.py	2012-11-08 18:50:26 +0000
@@ -125,13 +125,18 @@
         d = query.submit()
         return d.addCallback(self.parser.truth_return)
 
-    def delete_security_group(self, name):
+    def delete_security_group(self, name=None, id=None):
         """
         @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}
+        if name:
+            parameter = {"GroupName":  name}
+        elif id:
+            parameter = {"GroupId": id}
+        else:
+            raise ValueError("You must provide either the security group name or id")
         query = self.query_factory(
             action="DeleteSecurityGroup", creds=self.creds,
             endpoint=self.endpoint, other_params=parameter)
@@ -670,6 +675,7 @@
         root = XML(xml_bytes)
         result = []
         for group_info in root.findall("securityGroupInfo/item"):
+            id = group_info.findtext("groupId")
             name = group_info.findtext("groupName")
             description = group_info.findtext("groupDescription")
             owner_id = group_info.findtext("ownerId")
@@ -709,7 +715,7 @@
                               for user_id, group_name in allowed_groups]
 
             security_group = model.SecurityGroup(
-                name, description, owner_id=owner_id,
+                id, name, description, owner_id=owner_id,
                 groups=allowed_groups, ips=allowed_ips)
             result.append(security_group)
         return result

=== modified file 'txaws/ec2/model.py'
--- txaws/ec2/model.py	2012-03-02 22:00:10 +0000
+++ txaws/ec2/model.py	2012-11-08 18:50:26 +0000
@@ -80,7 +80,8 @@
     @ivar allowed_ips: The sequence of L{IPPermission} instances for this
         security group.
     """
-    def __init__(self, name, description, owner_id="", groups=None, ips=None):
+    def __init__(self, id, name, description, owner_id="", groups=None, ips=None):
+        self.id = id
         self.name = name
         self.description = description
         self.owner_id = owner_id

=== modified file 'txaws/ec2/tests/test_client.py'
--- txaws/ec2/tests/test_client.py	2012-03-02 22:00:10 +0000
+++ txaws/ec2/tests/test_client.py	2012-11-08 18:50:26 +0000
@@ -400,6 +400,7 @@
 
         def check_results(security_groups):
             [security_group] = security_groups
+            self.assertEquals(security_group.id, "sg-a1a1a1")
             self.assertEquals(security_group.owner_id,
                               "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM")
             self.assertEquals(security_group.name, "WebServers")
@@ -440,6 +441,7 @@
             security_group = security_groups[0]
             self.assertEquals(security_group.owner_id,
                               "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM")
+            self.assertEquals(security_group.id, "sg-a1a1a1")
             self.assertEquals(security_group.name, "MessageServers")
             self.assertEquals(security_group.description, "Message Servers")
             self.assertEquals(security_group.allowed_groups, [])
@@ -451,6 +453,7 @@
             security_group = security_groups[1]
             self.assertEquals(security_group.owner_id,
                               "UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM")
+            self.assertEquals(security_group.id, "sg-c3c3c3")
             self.assertEquals(security_group.name, "WebServers")
             self.assertEquals(security_group.description, "Web Servers")
             self.assertEquals([(pair.user_id, pair.group_name)
@@ -590,7 +593,7 @@
             "The group for the web server farm.")
         return self.assertTrue(d)
 
-    def test_delete_security_group(self):
+    def test_delete_security_group_using_name(self):
         """
         L{EC2Client.delete_security_group} returns a C{Deferred} that
         eventually fires with a true value, indicating the success of the
@@ -615,6 +618,40 @@
         d = ec2.delete_security_group("WebServers")
         return self.assertTrue(d)
 
+    def test_delete_security_group_using_id(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=None, endpoint=None,
+                         other_params={}):
+                self.assertEqual(action, "DeleteSecurityGroup")
+                self.assertEqual(creds.access_key, "foo")
+                self.assertEqual(creds.secret_key, "bar")
+                self.assertEqual(other_params, {
+                    "GroupId": "sg-a1a1a1",
+                    })
+
+            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(id="sg-a1a1a1")
+        return self.assertTrue(d)
+
+    def test_delete_security_group_without_id_and_name(self):
+        creds = AWSCredentials("foo", "bar")
+        ec2 = client.EC2Client(creds)
+        error = self.assertRaises(ValueError, ec2.delete_security_group)
+        self.assertEquals(
+            str(error),
+            "You must provide either the security group name or id",
+        )
+
     def test_delete_security_group_failure(self):
         """
         L{EC2Client.delete_security_group} returns a C{Deferred} that

=== modified file 'txaws/ec2/tests/test_model.py'
--- txaws/ec2/tests/test_model.py	2012-01-23 01:04:25 +0000
+++ txaws/ec2/tests/test_model.py	2012-11-08 18:50:26 +0000
@@ -8,7 +8,8 @@
 class SecurityGroupTestCase(TXAWSTestCase):
 
     def test_creation_defaults(self):
-        group = model.SecurityGroup("name", "desc")
+        group = model.SecurityGroup("sg-a3f2", "name", "desc")
+        self.assertEquals(group.id, "sg-a3f2")
         self.assertEquals(group.name, "name")
         self.assertEquals(group.description, "desc")
         self.assertEquals(group.owner_id, "")
@@ -18,14 +19,15 @@
     def test_creation_all_parameters(self):
         user = "somegal24"
         other_groups = [
-            model.SecurityGroup("other1", "another group 1"),
-            model.SecurityGroup("other2", "another group 2")]
+            model.SecurityGroup("sg-other1", "other1", "another group 1"),
+            model.SecurityGroup("sg-other2", "other2", "another group 2")]
         user_group_pairs = [
             model.UserIDGroupPair(user, other_groups[0].name),
             model.UserIDGroupPair(user, other_groups[1].name)]
         ips = [model.IPPermission("tcp", "80", "80", "10.0.1.0/24")]
         group = model.SecurityGroup(
-            "name", "desc", owner_id="me", groups=user_group_pairs, ips=ips)
+            "id", "name", "desc", owner_id="me", groups=user_group_pairs, ips=ips)
+        self.assertEquals(group.id, "id")
         self.assertEquals(group.name, "name")
         self.assertEquals(group.description, "desc")
         self.assertEquals(group.owner_id, "me")

=== modified file 'txaws/testing/payload.py'
--- txaws/testing/payload.py	2012-05-16 02:47:12 +0000
+++ txaws/testing/payload.py	2012-11-08 18:50:26 +0000
@@ -213,6 +213,7 @@
          <fromPort/>
         </item>
       </ipPermissions>
+      <groupId>sg-a1a1a1</groupId>
       <groupName>WebServers</groupName>
       <groupDescription>Web servers</groupDescription>
       <ownerId>UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM</ownerId>
@@ -228,6 +229,7 @@
   <securityGroupInfo>
     <item>
       <ownerId>UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM</ownerId>
+      <groupId>sg-a1a1a1</groupId>
       <groupName>WebServers</groupName>
       <groupDescription>Web Servers</groupDescription>
       <ipPermissions>
@@ -256,6 +258,7 @@
   <securityGroupInfo>
     <item>
       <ownerId>UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM</ownerId>
+      <groupId>sg-a1a1a1</groupId>
       <groupName>MessageServers</groupName>
       <groupDescription>Message Servers</groupDescription>
       <ipPermissions>
@@ -274,6 +277,7 @@
     </item>
     <item>
       <ownerId>UYY3TLBUXIEON5NQVUUX6OMPWBZIQNFM</ownerId>
+      <groupId>sg-c3c3c3</groupId>
       <groupName>WebServers</groupName>
       <groupDescription>Web Servers</groupDescription>
       <ipPermissions>