security groups filtered with tenant_id does not accept rbac

[Jesper Schmitz Mouridsen]

Horizon filters security groups on tenant_id, this excludes rbac shared security groups from other projects/tenants. Perhaps there should be a filter option called tenant_allowed so that horizon can filter correctly for all tenants, without losing the ability to filter on owner project_id.

[Jesper Schmitz Mouridsen (jsmdk)]

the two below patches uses neutron_lib/db/model_query.py around line 206

     elif key == 'shared' and hasattr(model, 'rbac_entries'):
                # translate a filter on shared into a query against the
                # object's rbac entries
                rbac = model.rbac_entries.property.mapper.class_
                matches = [rbac.target_tenant == '*']
                if context:
                    matches.append(rbac.target_tenant == context.tenant_id)
                # any 'access_as_shared' records that match the
                # wildcard or requesting tenant
                is_shared = and_(rbac.action == 'access_as_shared',
                                 or_(*matches))

so adding a shared filter does return shared security groups, but horizon needs two calls,
is there an elegant way to merge the two queries?

Here goes the patches

diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py
index 7bd3e29fb..9d88d9692 100644
--- a/openstack_dashboard/api/neutron.py
+++ b/openstack_dashboard/api/neutron.py
@@ -355,7 +355,11 @@ class SecurityGroupManager(object):

     def _list(self, **filters):
         secgroups = self.client.list_security_groups(**filters)
- return [SecurityGroup(sg) for sg in secgroups.get('security_groups')]
+ if filters.get("tenant_id"):
+ filters.pop("tenant_id")
+ filters["shared"]=True
+ secgroups_rbac = self.client.list_security_groups(**filters)
+ return [SecurityGroup(sg) for sg in secgroups.get('security_groups') + secgroups_rbac.get("security_groups")]

     @profiler.trace
     def list(self, **params):

diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py
index 48b3b5f4c0..ee4a05a650 100644
--- a/neutron/extensions/securitygroup.py
+++ b/neutron/extensions/securitygroup.py
@@ -233,6 +233,9 @@ RESOURCE_ATTRIBUTE_MAP = {
                       'validate': {
                           'type:string': db_const.PROJECT_ID_FIELD_SIZE},
                       'is_visible': True, 'is_filter': True},
+ 'shared' : { 'allow_post': False, 'allow_put': False,
+ 'is_sort_key': False,
+ 'is_filter': True, 'primary_key': False},
         SECURITYGROUPRULES: {'allow_post': False, 'allow_put': False,
                              'is_visible': True},
     },
diff --git a/neutron/objects/securitygroup.py b/neutron/objects/securitygroup.py
index 5edfd80b98..370683341f 100644
--- a/neutron/objects/securitygroup.py
+++ b/neutron/objects/securitygroup.py
@@ -62,7 +62,7 @@ class SecurityGroup(rbac_db.NeutronRbacObject):

     synthetic_fields = ['is_default', 'rules']

- extra_filter_names = {'is_default'}
+ extra_filter_names = {'is_default','shared'}

     lazy_fields = set(['rules'])

[Slawek Kaplonski (slaweq)]

I just tested it locally with Openstack CLI client and it works fine:

http://::1:9696 "GET /v2.0/security-groups?fields=id&fields=name&fields=description&fields=project_id&fields=tags HTTP/1.1" 200 341
RESP: [200] Connection: keep-alive Content-Length: 341 Content-Type: application/json Date: Mon, 14 Dec 2020 11:46:03 GMT X-Openstack-Request-Id: req-778a68db-dc59-4323-b5c8-6cccb5475703
RESP BODY: {"security_groups": [{"id": "1fc10a43-4e50-45ef-b82e-7eacc1e76414", "name": "test-rbac", "description": "", "tags": [], "project_id": "90faad0a19974fa39d33a875cb9ba84d"}, {"id": "64e7e48b-09a1-4d7b-86e7-d7bdc8ac4dab", "name": "default", "description": "Default security group", "tags": [], "project_id": "40c8b13e29404b8492c8099df7525a79"}]}
GET call to network for http://[::1]:9696/v2.0/security-groups?fields=id&fields=name&fields=description&fields=project_id&fields=tags used request id req-778a68db-dc59-4323-b5c8-6cccb5475703
+--------------------------------------+-----------+------------------------+----------------------------------+------+
| ID | Name | Description | Project | Tags |
+--------------------------------------+-----------+------------------------+----------------------------------+------+
| 1fc10a43-4e50-45ef-b82e-7eacc1e76414 | test-rbac | | 90faad0a19974fa39d33a875cb9ba84d | [] |
| 64e7e48b-09a1-4d7b-86e7-d7bdc8ac4dab | default | Default security group | 40c8b13e29404b8492c8099df7525a79 | [] |
+--------------------------------------+-----------+------------------------+----------------------------------+------+

Based on that I think that it isn't Neutron bug but maybe something in Horizon. I'm marking this bug as Incomplete in Neutron for now and I will also add Horizon as affected project.

[Slawek Kaplonski (slaweq)]

Sorry, not "Incomplete" but "Invalid" in Neutron

[Jesper Schmitz Mouridsen (jsmdk)]

But if you filter on tenant_id as horizon does you will not see your test-rbac group.
Horizon "has to" filter on tenant_id otherwise admin does list security groups for all projects.
But okay, probably mostly a horizon issue..

[Akihiro Motoki (amotoki)]

This is a known feature gap in horizon side and it has not been implemented just due to the lack of contributors.

The following is an advise to someone who would like to implement the security group RBAC feature in horizon.

Horizon needs to consider a scoped project has "admin" role. Neutron API returns all security groups from all tenants (i.e. projects) if the API is called with a token with "admin" role so filtering with tenant_id is required to cope with the case (openstack_dashboard/api/neutron.py SecurityGroupManager._list).
To support security group RBAC, you need to consider both "shared" attribute in security group and a user with "admin" role. (If neutron API behaves like nova API when a user has "admin" role, horizon implementation would be much simpler, but the real is not ...)
Perhpas the implementation of network_list would be a good reference.

[Akihiro Motoki (amotoki)]

neutron API with "admin" role returns all resources from all projects. This has not been changed since it was implemented, so it is not easy to change this behavior. Horizon needs to cope with the situation.
openstack_dashboard/api/neutron.py: list_networks_for_tenant() is a workaround on this neutron API behavior [1].
All neutron RBAC support in horizon would need to have this kind of workarounds.
https://opendev.org/openstack/horizon/src/branch/master/openstack_dashboard/api/neutron.py#L1055-L1101

[Jesper Schmitz Mouridsen (jsmdk)]

So my first attempt was not that far off..
But GET //v2.0/security-groups?shared=True HTTP/1.1" status: 400
and ['shared'] is invalid attribute for filtering Neutron server returns request_ids: ['req-d2b607dd-b282-477c-81c7-8debd3cf7d30']

So that filter should be implemented in neutron at first in order for horizon to make use of RBAC for security groups?

As my original suggestion:

diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py
index e212f4baf1..bee72ffd52 100644
--- a/neutron/extensions/securitygroup.py
+++ b/neutron/extensions/securitygroup.py
@@ -233,6 +233,9 @@ RESOURCE_ATTRIBUTE_MAP = {
                       'validate': {
                           'type:string': db_const.PROJECT_ID_FIELD_SIZE},
                       'is_visible': True, 'is_filter': True},
+ 'shared' : { 'allow_post': False, 'allow_put': False,
+ 'is_sort_key': False,
+ 'is_filter': True, 'primary_key': False},
         SECURITYGROUPRULES: {'allow_post': False, 'allow_put': False,
                              'is_visible': True},
     },

diff --git a/openstack_dashboard/api/neutron.py b/openstack_dashboard/api/neutron.py
index 7bd3e29fb..9d88d9692 100644
--- a/openstack_dashboard/api/neutron.py
+++ b/openstack_dashboard/api/neutron.py
@@ -355,7 +355,11 @@ class SecurityGroupManager(object):

     def _list(self, **filters):
         secgroups = self.client.list_security_groups(**filters)
- return [SecurityGroup(sg) for sg in secgroups.get('security_groups')]
+ if filters.get("tenant_id"):
+ filters.pop("tenant_id")
+ filters["shared"]=True
+ secgroups_rbac = self.client.list_security_groups(**filters)
+ return [SecurityGroup(sg) for sg in secgroups.get('security_groups') + secgroups_rbac.get("security_groups")]

     @profiler.trace
     def list(self, **params):

[Hang Yang (hangyang)]

I ran into the same issue and have verified Jesper's idea can work. I'm working on the Neutron and Horizon patches. Related Neutron bug: https://bugs.launchpad.net/neutron/+bug/1942615

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/horizon/+/811520

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/horizon/+/811520
Committed: https://opendev.org/openstack/horizon/commit/c7ea66bc3ede39fcc2d341312c741d662dc4cf7e
Submitter: "Zuul (22348)"
Branch: master

commit c7ea66bc3ede39fcc2d341312c741d662dc4cf7e
Author: Hang Yang <email address hidden>
Date: Tue Sep 28 20:23:38 2021 -0500

    Support RBAC security groups in dashboard

    Get the RBAC shared security groups in the dashboard by making
    an additional Neutron API call to filter by the shared field. Currently,
    the dashboard only shows SGs owned by the tenant.

    Depends-On: https://review.opendev.org/c/openstack/neutron/+/811242
    Closes-Bug: #1907843
    Change-Id: Ifa1acb3f0f6a33d0b4dc3761674e561a8d24c5c2

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/horizon 21.0.0 release.