Create and delete security groups works.

Adding and revoking rules works.
DescribeSecurityGroups returns the groups and rules.
So, the API seems to be done. Yay.

6 files changed, 104 insertions, 16 deletions

diff --git a/nova/db/api.py b/nova/db/api.py
index 63ead04e0..c7a6da183 100644
--- a/nova/db/api.py
+++ b/nova/db/api.py
@@ -451,6 +451,11 @@ def security_group_create(context, values):
     return IMPL.security_group_create(context, values)
 
 
+def security_group_get_by_id(context, security_group_id):
+    """Get security group by its internal id"""
+    return IMPL.security_group_get_by_id(context, security_group_id)
+
+
 def security_group_get_by_instance(context, instance_id):
     """Get security groups to which the instance is assigned"""
     return IMPL.security_group_get_by_instance(context, instance_id)
diff --git a/nova/db/sqlalchemy/api.py b/nova/db/sqlalchemy/api.py
index 2db876154..4027e901c 100644
--- a/nova/db/sqlalchemy/api.py
+++ b/nova/db/sqlalchemy/api.py
@@ -595,6 +595,12 @@ def security_group_create(_context, values):
     return security_group_ref
 
 
+def security_group_get_by_id(_context, security_group_id):
+    with managed_session() as session:
+        return session.query(models.SecurityGroup) \
+                      .get(security_group_id)
+
+
 def security_group_get_by_instance(_context, instance_id):
     with managed_session() as session:
         return session.query(models.Instance) \
@@ -608,6 +614,7 @@ def security_group_get_by_user(_context, user_id):
         return session.query(models.SecurityGroup) \
                              .filter_by(user_id=user_id) \
                              .filter_by(deleted=False) \
+                             .options(eagerload('rules')) \
                              .all()
 
 def security_group_get_by_user_and_name(_context, user_id, name):
diff --git a/nova/db/sqlalchemy/models.py b/nova/db/sqlalchemy/models.py
index d177688d8..27c8e4d4c 100644
--- a/nova/db/sqlalchemy/models.py
+++ b/nova/db/sqlalchemy/models.py
@@ -329,8 +329,8 @@ class SecurityGroupIngressRule(BASE, NovaBase):
     id = Column(Integer, primary_key=True)
 
     parent_group_id = Column(Integer, ForeignKey('security_group.id'))
-    parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id)
-#                                primaryjoin=SecurityGroup().id==parent_group_id)
+    parent_group = relationship("SecurityGroup", backref="rules", foreign_keys=parent_group_id,
+                                  primaryjoin=parent_group_id==SecurityGroup.id)
 
     protocol = Column(String(5)) # "tcp", "udp", or "icmp"
     from_port = Column(Integer)
@@ -338,7 +338,7 @@ class SecurityGroupIngressRule(BASE, NovaBase):
 
     # Note: This is not the parent SecurityGroup. It's SecurityGroup we're
     # granting access for.
-#    group_id = Column(Integer, ForeignKey('security_group.id'))
+    group_id = Column(Integer, ForeignKey('security_group.id'))
 
     @property
     def user(self):
diff --git a/nova/endpoint/api.py b/nova/endpoint/api.py
index 40be00bb7..1f37aeb02 100755
--- a/nova/endpoint/api.py
+++ b/nova/endpoint/api.py
@@ -135,6 +135,7 @@ class APIRequest(object):
 
         response = xml.toxml()
         xml.unlink()
+#        print response
         _log.debug(response)
         return response
 
diff --git a/nova/endpoint/cloud.py b/nova/endpoint/cloud.py
index 6e32a945b..e6eca9850 100644
--- a/nova/endpoint/cloud.py
+++ b/nova/endpoint/cloud.py
@@ -213,14 +213,41 @@ class CloudController(object):
 
     @rbac.allow('all')
     def describe_security_groups(self, context, **kwargs):
-        groups = {'securityGroupSet': 
-                  [{ 'groupDescription': group.description,
-                     'groupName' : group.name,
-                     'ownerId': context.user.id } for group in \
-                         db.security_group_get_by_user(context,
-                                                       context.user.id) ] }
-
-        return groups
+        groups = []
+        for group in db.security_group_get_by_user(context, context.user.id):
+            group_dict = {}
+            group_dict['groupDescription'] = group.description
+            group_dict['groupName'] = group.name
+            group_dict['ownerId'] = context.user.id
+            group_dict['ipPermissions'] = []
+            for rule in group.rules:
+                rule_dict = {}
+                rule_dict['ipProtocol'] = rule.protocol
+                rule_dict['fromPort'] = rule.from_port
+                rule_dict['toPort'] = rule.to_port
+                rule_dict['groups'] = []
+                rule_dict['ipRanges'] = []
+                if rule.group_id:
+                    foreign_group = db.security_group_get_by_id({}, rule.group_id)
+                    rule_dict['groups'] += [ { 'groupName': foreign_group.name,
+                                              'userId': foreign_group.user_id } ]
+                else:
+                    rule_dict['ipRanges'] += [ { 'cidrIp': rule.cidr } ]
+                group_dict['ipPermissions'] += [ rule_dict ]
+            groups += [ group_dict ]
+
+        return {'securityGroupInfo': groups }
+#
+#                 [{ 'groupDescription': group.description,
+#                     'groupName' : group.name,
+#                     'ownerId': context.user.id,
+#                     'ipPermissions' : [
+#                        { 'ipProtocol' : rule.protocol,
+#                          'fromPort' : rule.from_port,
+#                          'toPort' : rule.to_port,
+#                          'ipRanges' : [ { 'cidrIp' : rule.cidr } ] } for rule in group.rules ] } for group in \
+#
+#        return groups
 
     @rbac.allow('netadmin')
     def revoke_security_group_ingress(self, context, group_name,
diff --git a/nova/tests/api_unittest.py b/nova/tests/api_unittest.py
index f25e377d0..7e914e6f5 100644
--- a/nova/tests/api_unittest.py
+++ b/nova/tests/api_unittest.py
@@ -293,19 +293,43 @@ class ApiEc2TestCase(test.BaseTestCase):
         self.mox.ReplayAll()
         group.connection = self.ec2
 
-        group.authorize('tcp', 80, 80, '0.0.0.0/0')
+        group.authorize('tcp', 80, 81, '0.0.0.0/0')
+
+        self.expect_http()
+        self.mox.ReplayAll()
+
+        rv = self.ec2.get_all_security_groups()
+        # I don't bother checkng that we actually find it here,
+        # because the create/delete unit test further up should 
+        # be good enough for that.
+        for group in rv:
+            if group.name == security_group_name:
+                self.assertEquals(len(group.rules), 1)
+                self.assertEquals(int(group.rules[0].from_port), 80)
+                self.assertEquals(int(group.rules[0].to_port), 81)
+                self.assertEquals(len(group.rules[0].grants), 1)
+                self.assertEquals(str(group.rules[0].grants[0]),  '0.0.0.0/0')
 
         self.expect_http()
         self.mox.ReplayAll()
         group.connection = self.ec2
 
-        group.revoke('tcp', 80, 80, '0.0.0.0/0')
+        group.revoke('tcp', 80, 81, '0.0.0.0/0')
 
         self.expect_http()
         self.mox.ReplayAll()
 
         self.ec2.delete_security_group(security_group_name)
 
+        self.expect_http()
+        self.mox.ReplayAll()
+        group.connection = self.ec2
+
+        rv = self.ec2.get_all_security_groups()
+
+        self.assertEqual(len(rv), 1)
+        self.assertEqual(rv[0].name, 'default')
+
         self.manager.delete_project(project)
         self.manager.delete_user(user)
 
@@ -323,13 +347,16 @@ class ApiEc2TestCase(test.BaseTestCase):
 
         security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \
                                       for x in range(random.randint(4, 8)))
+        other_security_group_name = "".join(random.choice("sdiuisudfsdcnpaqwertasd") \
+                                          for x in range(random.randint(4, 8)))
 
         group = self.ec2.create_security_group(security_group_name, 'test group')
 
         self.expect_http()
         self.mox.ReplayAll()
 
-        other_group = self.ec2.create_security_group('appserver', 'The application tier')
+        other_group = self.ec2.create_security_group(other_security_group_name,
+                                                     'some other group')
 
         self.expect_http()
         self.mox.ReplayAll()
@@ -339,9 +366,30 @@ class ApiEc2TestCase(test.BaseTestCase):
 
         self.expect_http()
         self.mox.ReplayAll()
-        group.connection = self.ec2
 
-        group.revoke(src_group=other_group)
+        rv = self.ec2.get_all_security_groups()
+        # I don't bother checkng that we actually find it here,
+        # because the create/delete unit test further up should 
+        # be good enough for that.
+        for group in rv:
+            if group.name == security_group_name:
+                self.assertEquals(len(group.rules), 1)
+                self.assertEquals(len(group.rules[0].grants), 1)
+                self.assertEquals(str(group.rules[0].grants[0]),
+                                  '%s-%s' % (other_security_group_name, 'fake'))
+
+
+        self.expect_http()
+        self.mox.ReplayAll()
+
+        rv = self.ec2.get_all_security_groups()
+
+        for group in rv:
+            if group.name == security_group_name:
+                self.expect_http()
+                self.mox.ReplayAll()
+                group.connection = self.ec2
+                group.revoke(src_group=other_group)
 
         self.expect_http()
         self.mox.ReplayAll()