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/<project_id>/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

@@ -431,10 +431,13 @@ def delete_key_pair(self, context, key_name, **kwargs):
 
     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]
 

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 {}

nova/api/openstack/compute/contrib/security_groups.py

@@ -266,8 +266,12 @@ def index(self, req):
         """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)

nova/compute/api.py

@@ -2281,7 +2281,8 @@ def get(self, context, name=None, id=None, map_exception=False):
             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 @@ def list(self, context, names=None, ids=None, project=None):
                     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)

nova/tests/api/ec2/test_cloud.py

@@ -301,6 +301,38 @@ def test_describe_security_groups(self):
                 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,

nova/tests/api/openstack/compute/contrib/test_security_groups.py

@@ -291,6 +291,45 @@ def return_security_groups(context, project_id):
 
         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']):