From 29af2252a8bc97157a52fddca78b31224eb55dac Mon Sep 17 00:00:00 2001 From: Eoghan Glynn Date: Fri, 14 Sep 2012 11:15:29 +0000 Subject: All security groups not returned to admins by default. Fixes bug 1046054. Previously security groups relating to all tenants were returned when requested by an admin user. Now only those groups related to the current tenant are returned by default. To recover the old behaviour, the all_tenants search option may be specified via the native API with: /v2//os-security-groups?all_tenants=1 or via the EC2 API with: Action=DescribeSecurityGroups&Filter.1.Name=all-tenants&Filter.1.Value.1=1 Note that the latter is slightly ultra vires with respect to the EC2 API spec, in the sense that this filter is in addition to the standard set. Since we don't pay attention to many of these standard filters as yet, this stepping slightly off-piste is deemed worth it. Change-Id: I6157e408394d04096d21747d665e3b3aa6aa55de --- nova/api/ec2/cloud.py | 5 ++- nova/api/ec2/ec2utils.py | 5 +++ .../openstack/compute/contrib/security_groups.py | 6 +++- nova/compute/api.py | 12 +++++-- nova/tests/api/ec2/test_cloud.py | 32 ++++++++++++++++++ .../compute/contrib/test_security_groups.py | 39 ++++++++++++++++++++++ 6 files changed, 95 insertions(+), 4 deletions(-) diff --git a/nova/api/ec2/cloud.py b/nova/api/ec2/cloud.py index 0574ac262..0456dbc2a 100644 --- a/nova/api/ec2/cloud.py +++ b/nova/api/ec2/cloud.py @@ -431,10 +431,13 @@ class CloudController(object): def describe_security_groups(self, context, group_name=None, group_id=None, **kwargs): + search_opts = ec2utils.search_opts_from_filters(kwargs.get('filter')) + raw_groups = self.security_group_api.list(context, group_name, group_id, - context.project_id) + context.project_id, + search_opts=search_opts) groups = [self._format_security_group(context, g) for g in raw_groups] diff --git a/nova/api/ec2/ec2utils.py b/nova/api/ec2/ec2utils.py index 4a7e574ad..fdff3d9f4 100644 --- a/nova/api/ec2/ec2utils.py +++ b/nova/api/ec2/ec2utils.py @@ -301,3 +301,8 @@ def dict_from_dotted_str(items): args[key] = value return args + + +def search_opts_from_filters(filters): + return dict((f['name'].replace('-', '_'), f['value']['1']) + for f in filters if f['value']['1']) if filters else {} diff --git a/nova/api/openstack/compute/contrib/security_groups.py b/nova/api/openstack/compute/contrib/security_groups.py index 8bb9f3cf1..e5b1797b4 100644 --- a/nova/api/openstack/compute/contrib/security_groups.py +++ b/nova/api/openstack/compute/contrib/security_groups.py @@ -266,8 +266,12 @@ class SecurityGroupController(SecurityGroupControllerBase): """Returns a list of security groups""" context = self._authorize_context(req) + search_opts = {} + search_opts.update(req.GET) + raw_groups = self.security_group_api.list(context, - project=context.project_id) + project=context.project_id, + search_opts=search_opts) limited_list = common.limited(raw_groups, req) result = [self._format_security_group(context, group) diff --git a/nova/compute/api.py b/nova/compute/api.py index c4476edb0..0b82d0ed5 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -2281,7 +2281,8 @@ class SecurityGroupAPI(base.Base): else: raise - def list(self, context, names=None, ids=None, project=None): + def list(self, context, names=None, ids=None, project=None, + search_opts=None): self.ensure_default(context) groups = [] @@ -2296,7 +2297,14 @@ class SecurityGroupAPI(base.Base): groups.append(self.db.security_group_get(context, id)) elif context.is_admin: - groups = self.db.security_group_get_all(context) + # TODO(eglynn): support a wider set of search options than just + # all_tenants, at least include the standard filters defined for + # the EC2 DescribeSecurityGroups API for the non-admin case also + if (search_opts and 'all_tenants' in search_opts): + groups = self.db.security_group_get_all(context) + else: + groups = self.db.security_group_get_by_project(context, + project) elif project: groups = self.db.security_group_get_by_project(context, project) diff --git a/nova/tests/api/ec2/test_cloud.py b/nova/tests/api/ec2/test_cloud.py index 6301ce180..a444213d3 100644 --- a/nova/tests/api/ec2/test_cloud.py +++ b/nova/tests/api/ec2/test_cloud.py @@ -301,6 +301,38 @@ class CloudTestCase(test.TestCase): sec['name']) db.security_group_destroy(self.context, sec['id']) + def test_describe_security_groups_all_tenants(self): + """Makes sure describe_security_groups works and filters results.""" + sec = db.security_group_create(self.context, + {'project_id': 'foobar', + 'name': 'test'}) + + def _check_name(result, i, expected): + self.assertEqual(result['securityGroupInfo'][i]['groupName'], + expected) + + # include all tenants + filter = [{'name': 'all-tenants', 'value': {'1': 1}}] + result = self.cloud.describe_security_groups(self.context, + filter=filter) + self.assertEqual(len(result['securityGroupInfo']), 2) + _check_name(result, 0, 'default') + _check_name(result, 1, sec['name']) + + # exclude all tenants + filter = [{'name': 'all-tenants', 'value': {'1': 0}}] + result = self.cloud.describe_security_groups(self.context, + filter=filter) + self.assertEqual(len(result['securityGroupInfo']), 1) + _check_name(result, 0, 'default') + + # default all tenants + result = self.cloud.describe_security_groups(self.context) + self.assertEqual(len(result['securityGroupInfo']), 1) + _check_name(result, 0, 'default') + + db.security_group_destroy(self.context, sec['id']) + def test_describe_security_groups_by_id(self): sec = db.security_group_create(self.context, {'project_id': self.context.project_id, diff --git a/nova/tests/api/openstack/compute/contrib/test_security_groups.py b/nova/tests/api/openstack/compute/contrib/test_security_groups.py index f32b688d9..22973ed66 100644 --- a/nova/tests/api/openstack/compute/contrib/test_security_groups.py +++ b/nova/tests/api/openstack/compute/contrib/test_security_groups.py @@ -291,6 +291,45 @@ class TestSecurityGroups(test.TestCase): self.assertEquals(res_dict, expected) + def test_get_security_group_list_all_tenants(self): + all_groups = [] + tenant_groups = [] + + for i, name in enumerate(['default', 'test']): + sg = security_group_template(id=i + 1, + name=name, + description=name + '-desc', + rules=[]) + all_groups.append(sg) + if name == 'default': + tenant_groups.append(sg) + + all = {'security_groups': all_groups} + tenant_specific = {'security_groups': tenant_groups} + + def return_all_security_groups(context): + return [security_group_db(sg) for sg in all_groups] + + self.stubs.Set(nova.db, 'security_group_get_all', + return_all_security_groups) + + def return_tenant_security_groups(context, project_id): + return [security_group_db(sg) for sg in tenant_groups] + + self.stubs.Set(nova.db, 'security_group_get_by_project', + return_tenant_security_groups) + + path = '/v2/fake/os-security-groups' + + req = fakes.HTTPRequest.blank(path, use_admin_context=True) + res_dict = self.controller.index(req) + self.assertEquals(res_dict, tenant_specific) + + req = fakes.HTTPRequest.blank('%s?all_tenants=1' % path, + use_admin_context=True) + res_dict = self.controller.index(req) + self.assertEquals(res_dict, all) + def test_get_security_group_by_instance(self): groups = [] for i, name in enumerate(['default', 'test']): -- cgit