[RFE] Neutron quota api should be configurable via policy.json

[Divya K Konoor]

Neutron does not have rbac rule support for quota in neutron policy.json >> https://github.com/openstack/neutron/blob/master/etc/policy.json . The rbac validations are programmatically hardcoded in the neutron quota api flow >> https://github.com/openstack/neutron/blob/master/neutron/pecan_wsgi/controllers/quota.py . For this reason, we currently do not have a mechanism to configure this in neutron policy.json.

All REST api CRUD calls should have role based access control in place and OpenStack uses oslo_policy and policy.json files for this. There are rbac rules that are defined in the policy.json (one for each REST api CRUD operation) that can be used to configure the roles that can access the REST api. The neutron quota REST api however does not have this in place. For eg. cinder policy.json has the below rules that can be used to configure RBAC on cinder quotas:

    "volume_extension:quotas:show": "",
    "volume_extension:quotas:update": "rule:admin_api",
    "volume_extension:quotas:delete": "rule:admin_api",

https://github.com/openstack/cinder/blob/master/etc/cinder/policy.json#L44

[Ihar Hrachyshka (ihar-hrachyshka)]

Please describe exactly what you try to achieve here. Do you want to limit the number of RBAC policy resources per tenant? Or do you want to be able to expose some API requests that are currently not exposable? Which of them then?

[Divya K Konoor (dikonoor)]

Reedip , I have updated the description of the defect with more details on the problem reported here.

[Ihar Hrachyshka (ihar-hrachyshka)]

It sounds like a good isolated request. The starting point would be: https://github.com/openstack/neutron/blob/master/neutron/pecan_wsgi/controllers/quota.py#L60

[Reedip (reedip-banerjee-deactivatedaccount)]

Would like to take it up once Driver's meetup has finalized this

[Divya K Konoor (dikonoor)]

Reedip, What are finalized in the driver's meetup on this defect ?

[Reedip (reedip-banerjee-deactivatedaccount)]

Hi Divya,
This is still pending the proper driver team approval.

[Ihar Hrachyshka (ihar-hrachyshka)]

I agree with the proposal, seems like a well isolated and reasonable thing to do. I would even say that's a bug and not a RFE.

[Kevin Benton (kevinbenton)]

This is good to go. @reedip, please proceed directly with an implementation, this doesn't need a spec.

[Viktor Křivák (viktor-krivak)]

Hi guys,
it there some progress with this issue? It's kind of blocker for me so I offer help.
It looks like simple isolated change, so I could write this whole patch if needed. But I don't want to cause problems if someone is nearly done with this.

[Armando Migliaccio (armando-migliaccio)]

The lack of rules in policy.json means 'either the tenant or the admin'. I am not sure this is as simple as adding entries in the policy.json, especially when I read [1]. That said, can you elaborate why you would want to make this configurable?

[1] https://github.com/openstack/neutron/blame/master/neutron/pecan_wsgi/controllers/quota.py#L60

[Viktor Křivák (viktor-krivak)]

Hi,
lets presume following scenario. I need to create user with rights to define quotas for others. Quotas like max number of networks, routers etc. And also I don't want to give this user full access (for security reasons). So he doesn't see all objects from all projects.

This scenario cannot be configured because API just check admin_context via this code:
if not context.is_admin:
    raise n_exc.AdminRequired(reason=reason)

Implementation is actually here: https://github.com/openstack/neutron/blob/master/neutron/extensions/quotasv2.py

I try stupid approach just to verify if it working like I think (following code is very stupid, it is just a prove of concept)
change for example method update:

    def update(self, request, id, body=None):
        from neutron import policy
        context = request.context
        policy.init()
        if not policy.check(context, "update_quota", {'project_id': context.project_id, 'tenant_id': context.project_id}):
            raise n_exc.AdminRequired(reason="Only admin can view or configure quota")
        if self._update_extended_attributes:
            self._update_attributes()
        body = base.Controller.prepare_request_body(
            request.context, body, False, self._resource_name,
            EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION])
        for key, value in body[self._resource_name].items():
            self._driver.update_quota_limit(request.context, id, key, value)
        return {self._resource_name: self._get_quotas(request, id)}

Once again this is not final code, it is just a test if it can be done. You also need define line:
"update_quota": ""
in your policy.json
After this anyone can change quota (or you need better policy).

PS: I don't attach this code as patch because it is not a patch.

[Reedip (reedip-banerjee-deactivatedaccount)]

@Viktor: Hi,
Sorry , I skipped this issue for sometime.
But your fix seems to be correct, but the implementation can be changed to the pecan_wsgi/controllers/quota.py

I think the modification there would be more suitable.

[Viktor Křivák (viktor-krivak)]

@reedip: Hi, thanks for response. OK, I start working on this. I just want to make sure, that I don't screw up with someone else work.
OK I use pecan implementation and see what I can do.

[Reedip (reedip-banerjee-deactivatedaccount)]

I have a patch , but test cases remain.
Maybe you can assist me in the same, or maybe add up in the current patch impelementation which I have prepared ?

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/505155

[Viktor Křivák (viktor-krivak)]

Sorry for delay. It's look like quota API don't use pecan, but legacy controller instead. It was introduced in this commit https://github.com/openstack/neutron/commit/5cb73f5f4694e077c9bb0a3bf65f04d4c32fe3b5#diff-350062450daf0d63ec1c37a60c552f32

To be honest I don't fully understand why this change was needed but it result in using legacy quota controller and code in pecan_wsgi/controllers/quota.py is never actually run.
I think we can rewrite booth legacy and pecan controller and wait if pecan controller was used again.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/507446

[Viktor Křivák (viktor-krivak)]

Hi,
I sent alternative patchset. I edit both controllers. Legacy which is actually used and new pecan which is not used at this moment but in future can be again.
It add new policy:
- update_quota
- get_quota
- delete_quota
- default_quota

It is little dirty because I import code across controller but otherwise I need to duplicate code.
Gerrit somehow assign this issue to me, I don't want to but it somehow happen.

[Andrew Bogott (andrewbogott)]

I also need this fixed :) I want to make quota usage publicly visible to my users for reporting &c. and it's weird that read-only APIs are behind an admin check.

Furthermore, my understanding is that the keystone team has fully eliminated the concept of a binary admin and everything should be handled via specific policies. So this is a weird hanger-on which will surely break in all kinds of interesting ways in the future.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Reedip (<email address hidden>) on branch: master
Review: https://review.openstack.org/505155

[David Moreau Simard (dmsimard)]

I see https://review.openstack.org/505155 has been abandoned in favor of https://review.opendev.org/#/c/507446 but there hasn't been activity on it for many months.

Any chances for an update ?

We would like to delegate quota management to another team through a role.

We've been able to do it for quotas in Nova and Cinder with the following policies (on top of some additional policies for keystone):

==
{
    "quota_mgmt": "role:quota_mgmt",
    "os_compute_api:os-quota-sets:show": "rule:quota_mgmt or rule:admin_or_owner",
    "os_compute_api:os-quota-sets:detail": "rule:quota_mgmt or rule:admin_or_owner",
    "os_compute_api:os-quota-sets:update": "rule:quota_mgmt or rule:admin_api"
}
==
{
    "quota_mgmt": "role:quota_mgmt",
    "volume_extension:quotas:show": "rule:quota_mgmt or rule:admin_or_owner",
    "volume_extension:quotas:update": "rule:quota_mgmt or rule:admin_api"
}
==

This allows for commands such as "openstack quota list --compute" and "openstack quota list --volume" to return successfully but "openstack quota show" on another project will fail with an error from neutron:

No Quota found for <project_id>: Client Error for url: https://api:9696/v2.0/quotas/<project_id>, User does not have admin privileges: Only admin is authorized to access quotas for another tenant.

This is because, as the bug points out, the check is hardcoded here: https://github.com/openstack/neutron/blob/885cce39b7ad45d229a10c0378336cbbe125a94b/neutron/extensions/quotasv2.py#L72-L76

[Slawek Kaplonski (slaweq)]

Hi David,

I don't think there is any activity on this RFE now. RFE is approved so if You need it and want to implement that, feel free to work on this by continue already existing patches or send new ones.

[Andrew Bogott (andrewbogott)]

That last patch is mine. It was my understaning that https://review.opendev.org/#/c/507446 is awaiting review -- is that incorrect? We had discussed adding tests to that patch, but given that equivalent policy code doesn't currently have tests I wouldn't expect lack of tests to be a blocker.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/740999

[Andrew Bogott (andrewbogott)]

I added some basic tempest tests which should ensure that the new rbac code doesn't cause a regression. Would love a review of either patch.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/507446
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=c9242f9a889f4d69653de4d21bec6060f549ee7b
Submitter: Zuul
Branch: master

commit c9242f9a889f4d69653de4d21bec6060f549ee7b
Author: andrewbogott <email address hidden>
Date: Thu Dec 26 23:34:31 2019 -0600

    Allow RBAC on Neutron quotas

    This patch adds the support to allow role based access control
    on quota of resources.

    Change-Id: I6544d4a0794944abb3e1c2ff89134bf313cf35e8
    Closes-Bug: #1671448

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/ussuri
Review: https://review.opendev.org/c/openstack/neutron/+/828564

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/train
Review: https://review.opendev.org/c/openstack/neutron/+/828565

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/828564
Committed: https://opendev.org/openstack/neutron/commit/a20d904c8b1d577a59cfae0560fae749b72359f0
Submitter: "Zuul (22348)"
Branch: stable/ussuri

commit a20d904c8b1d577a59cfae0560fae749b72359f0
Author: andrewbogott <email address hidden>
Date: Thu Dec 26 23:34:31 2019 -0600

    Allow RBAC on Neutron quotas

    This patch adds the support to allow role based access control
    on quota of resources.

    Change-Id: I6544d4a0794944abb3e1c2ff89134bf313cf35e8
    Closes-Bug: #1671448
    (cherry picked from commit c9242f9a889f4d69653de4d21bec6060f549ee7b)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/neutron/+/828565
Committed: https://opendev.org/openstack/neutron/commit/d0b23d48892cf40d5b01df78ce8725137872d086
Submitter: "Zuul (22348)"
Branch: stable/train

commit d0b23d48892cf40d5b01df78ce8725137872d086
Author: andrewbogott <email address hidden>
Date: Thu Dec 26 23:34:31 2019 -0600

    Allow RBAC on Neutron quotas

    This patch adds the support to allow role based access control
    on quota of resources.

    Change-Id: I6544d4a0794944abb3e1c2ff89134bf313cf35e8
    Closes-Bug: #1671448
    (cherry picked from commit c9242f9a889f4d69653de4d21bec6060f549ee7b)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron train-eol release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron ussuri-eol release.