diff --git a/nova/db/api.py b/nova/db/api.py index 63ead04e0036..c7a6da183671 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 2db8761546ca..4027e901c2d8 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 d177688d8523..27c8e4d4ccf3 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 40be00bb7949..1f37aeb027df 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 6e32a945bc7d..e6eca98505ed 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) ] } + 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 groups + 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 f25e377d0c10..7e914e6f5690 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()