Nova should confirm quota requests against Keystone

[Scott Devoid]

os-quota-sets API should check requests for /v2/:tenant/os-quota-sets/ against Keystone to ensure that :tenant does exist.

POST requests to a non-existant tenant should fail with a 400 error code.

GET requests to a non-existant tenant may fail with a 400 error code. Current behavior is to return 200 with the default quotas. A slightly incompatible change would be to return a 302 redirect to /v2/:tenant/os-quota-sets/defaults in this case.

Edit (2014-01-22)

Original Description
--------------------
GET /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
returns 200 with the default quotas.

Moreover
POST /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
with updated quotas succeeds and that metadata is saved!

I'm not sure if this is a bug or not. I cannot find any documentation on this interface.

[Liyingjun (liyingjun)]

Similar bug: https://bugs.launchpad.net/python-novaclient/+bug/1031139

[melanie witt (melwitt)]

Can you show what novaclient commands you used to get the aforementioned behavior?

From the information provided, it's not clear whether you're using the client or calling the nova api directly.

[Scott Devoid (scott-devoid)]

This works if you make get or post requests against the os-quota-sets api.

On Wed, Dec 4, 2013 at 7:29 PM, Melanie Witt <email address hidden>wrote:

> Can you show what novaclient commands you used to get the aforementioned
> behavior?
>
> >From the information provided, it's not clear whether you're using the
> client or calling the nova api directly.
>
> ** Changed in: python-novaclient
> Status: Confirmed => Incomplete
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1118066
>
> Title:
> Request of quota sets for nonexistent tenant returns 200 with defaults
>
> Status in Python client library for Nova:
> Incomplete
>
> Bug description:
> GET /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
> returns 200 with the default quotas.
>
> Moreover
> POST /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
> with updated quotas succeeds and that metadata is saved!
>
> I'm not sure if this is a bug or not. I cannot find any documentation
> on this interface.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/python-novaclient/+bug/1118066/+subscriptions
>

[Scott Devoid (scott-devoid)]

Switched this to affecting nova itself. I can confirm that it is possible to insert quotas into the database for non-existent projects.

Steps to replicate:

$ keystone tenant-list | grep foobar # this is empty, indicating no 'foobar' tenant
$ nova quota-update foobar --cores 200
$ nova quota-show --tenant foobar
# Prints out the quotas

When I reported the issue it was affecting our Essex distribution. At that point the novaclient had programmatic support for quotas but no command line. Now, on Havana the command line has the same behavior as the API.

[Joe Gordon (jogo)]

So this is a known issue, nova doesn't do any tenant validation for quotas. Right now the assumption is that only global admins (think cloud operator) should have access to the last three methods in:

http://docs.openstack.org/api/openstack-compute/2/content/os-quota-sets.html

GET v2{/tenant_id}/os-quota-sets{/tenant_id}{/user_id}
Enables an admin user to show quotas for a specified tenant and user.

POST v2{/tenant_id}/os-quota-sets{/tenant_id}{/user_id}
Updates quotas for a specified tenant/project and user.

GET v2{/tenant_id}/os-quota-sets{/tenant_id}/detail{/user_id}
Shows details for quotas for a specified tenant and user.

And as an admin (trusted user), we expect them to not break things.

This is part of a bigger issue, which is nova doesn't have great RBAC support. Say you want to create a tenant admin who can set quotas per user.

[Scott Devoid (scott-devoid)]

"And as an admin (trusted user), we expect them to not break things."

Sorry, I am going to have to disagree with you on this. The interface gives no indication that the request failed to produce the desired effect. Add to that several facts: many quota-exceeded errors are masked by other quota exceeded error names and end users will report quota exceeded errors as "my instance failed to start". These all add up to a bad user experience.

"This is part of a bigger issue, which is nova doesn't have great RBAC support. Say you want to create a tenant admin who can set quotas per user."

I don't see how role-based access control is necessary when a simple check "does this string correspond to a real project UUID (or name if you want to support that)" would suffice.

Marking as open for these reasons.

[Joe Gordon (jogo)]

>"And as an admin (trusted user), we expect them to not break things."
>
>Sorry, I am going to have to disagree with you on this. The interface gives no indication that the request failed to produce the >desired effect. Add to that several facts: many quota-exceeded errors are masked by other quota exceeded error names and end >users will report quota exceeded errors as "my instance failed to start". These all add up to a bad user experience."

Yup, the UX is horrible for this one. can you expand on the error masking point?

>"This is part of a bigger issue, which is nova doesn't have great RBAC support. Say you want to create a tenant admin who can set >quotas per user."
>
>I don't see how role-based access control is necessary when a simple check "does this string correspond to a real project UUID (or >name if you want to support that)" would suffice.

So nova doesn't keep track of project UUIDs, so this would have to be implemented as a call out to keystone. So I am not very familiar with the keystone API but I think you would need to call v2.0/tenants{/tenantId} (http://docs.openstack.org/api/openstack-identity-service/2.0/content/Tenant_Operations.html) to make sure the tenant is valid or not.

>
>Marking as open for these reasons.

Perhaps opinion was the wrong status, while I agree that there is something to fix here, I am not sure how you want to change things. For this to be confirmed I would like a more explicit explanation of what the issue is and what the desired outcome should be. Do you just want to make sure the tenant is valid? If so I can get behind that, but the bug description needs some updating.

[Phil Day (philip-day)]

Goes beyond the scope of the specific bug here, but IMO the real fix for this kind of issue is that quota limits should be managed in Keystone (and passed to Nova and other services as part of the context) , and enforced in the service.

Project and User IDs are really just foreign keys to Nova, it shouldn't be managing properties of them.

[Scott Devoid (scott-devoid)]

> Yup, the UX is horrible for this one. can you expand on the error masking point?

That was an error I saw in Essex where one type of quota exception would be thrown when another type was exceeded. I'll check in our Havana setup and see if it is still a problem. I'll file a separate report if it is.

> I am not sure how you want to change things. For this to be confirmed I would like a more explicit explanation of what the issue is and what the desired outcome should be.

I think the simple fix is to have the nova api check against Keystone to validate the UUID before sending a response to the client. I'll update the bug description to suggest this solution.

[Scott Devoid (scott-devoid)]

Hi vaibhav, are you still working on this?

[Scott Devoid (scott-devoid)]

I would propose the following behavior:

When os-quota-sets is updated, nova-api checks the quota tables to see if the quota-set for the project ID already exists in the table. If it does exist, then update with the new quota value. Otherwise, use keystoneclient to confirm that the project ID exists. If it does not exist, return an appropriate error to the API. Otherwise update the new quota value.

This will catch the error except for cases where the quota table is already corrupted with quotas that apply to no projects.

[vaibhav (vaibhav-bhatkar)]

Hi Scott,

We fixed the bug keystone way in "Manila" but it was decided later that
the bug would not be fixed. Will send you the link of the fix tomorrow as
I am travelling today. BTW, keystoneclient is the only way to go as we
are not sure what keystone database is used in the background. Administrator
might have configured keystone to use LDAP.

Vaibhav

On Fri, Jun 13, 2014 at 9:44 PM, Scott Devoid <email address hidden> wrote:

> I would propose the following behavior:
>
> When os-quota-sets is updated, nova-api checks the quota tables to see
> if the quota-set for the project ID already exists in the table. If it
> does exist, then update with the new quota value. Otherwise, use
> keystoneclient to confirm that the project ID exists. If it does not
> exist, return an appropriate error to the API. Otherwise update the new
> quota value.
>
> This will catch the error except for cases where the quota table is
> already corrupted with quotas that apply to no projects.
>
> --
> You received this bug notification because you are a bug assignee.
> https://bugs.launchpad.net/bugs/1118066
>
> Title:
> Nova should confirm quota requests against Keystone
>
> Status in OpenStack Compute (Nova):
> Confirmed
>
> Bug description:
> os-quota-sets API should check requests for /v2/:tenant/os-quota-sets/
> against Keystone to ensure that :tenant does exist.
>
> POST requests to a non-existant tenant should fail with a 400 error
> code.
>
> GET requests to a non-existant tenant may fail with a 400 error code.
> Current behavior is to return 200 with the default quotas. A slightly
> incompatible change would be to return a 302 redirect to /v2/:tenant
> /os-quota-sets/defaults in this case.
>
> Edit (2014-01-22)
>
> Original Description
> --------------------
> GET /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
> returns 200 with the default quotas.
>
> Moreover
> POST /v2/:tenant/os-quota-sets/:this_tenant_does_not_exist
> with updated quotas succeeds and that metadata is saved!
>
> I'm not sure if this is a bug or not. I cannot find any documentation
> on this interface.
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/nova/+bug/1118066/+subscriptions
>

[Thang Pham (thang-pham)]

There is a blueprint to validate tenant and user IDs that is pending: https://blueprints.launchpad.net/nova/+spec/validate-tenant-user-with-keystone. It should resolve this bug and well as many other identical bugs.

[Davanum Srinivas (DIMS) (dims-v)]

There are no reviews, so this should not be in "In Progress"

[Joe Gordon (jogo)]

Just confirmed this is still an issue

[Joe Gordon (jogo)]

Moving priority up based on how many times this bug has been reported

[Mh Raies (raiesmh08)]

I will be working on this. And submit a review very soon.

[Mh Raies (raiesmh08)]

nova python client help tells that -

root@devstack:/opt/stack/nova# nova help quota-show
usage: nova quota-show [--tenant <tenant-id>] [--user <user-id>]

List the quotas for a tenant/user.

Optional arguments:
  --tenant <tenant-id> ID of tenant to list the quotas for.
  --user <user-id> ID of user to list the quotas for.
root@devstack:/opt/stack/nova#

Thus there is three possibilities -

1. Get quota filtered by user_id ==> get_user_quota
2. Get quota filtered by tenant_id ===> get_project_quota
3. Get quota filtered by user_id and tenant_id both ==> get_by_project_and_user

Current quota api implementation treats only "get_user_quota" or current project. Need to enhance/modify to entertain remainings.

[OpenStack Infra (hudson-openstack)]

Change abandoned by Joe Gordon (<email address hidden>) on branch: master
Review: https://review.openstack.org/143934
Reason: This review is > 4 weeks without comment and currently blocked by a core reviewer with a -2. We are abandoning this for now. Feel free to reactivate the review by pressing the restore button and contacting the reviewer with the -2 on this review to ensure you address their concerns.

[Hua Zhang (zhhuabj)]

Hi Mh, are you still working on this bug ?

[Mh Raies (raiesmh08)]

Hi Hua, Sorry to not give update on this.
Due to time constrains I could not go ahead with this.

Please, you can continue with this :)

[Matt Riedemann (mriedem)]

The link to the blueprint in comment 13 is wrong, it's this:

https://blueprints.launchpad.net/nova/+spec/validate-project-with-keystone

There was a change proposed for juno and kilo but missed deadlines:

https://review.openstack.org/#/c/143934/

Since that's an API impacting change it's going to require a spec for mitaka, which would also help to get operator and keystone team feedback. So I'm marking this as wishlist given it requires a spec.

[Matt Riedemann (mriedem)]

Rather than validate against keystone, couldn't we just fail if we're trying to update a quotas or project_quotas table entry where project_id doesn't exist?

[Matt Riedemann (mriedem)]

I validated today with openstackclient 1.7.0 that you can't show quotas for a project that doesn't exist:

stack@client:~/devstack$ openstack quota show randomtext
Tenant ID: randomtext does not exist. (HTTP 404) (Request-ID: req-cc9354ea-5935-45db-be60-b86aaf0a20ea)

And I validated that with the v2.1 API you can't request a quota update on non-standard resources:

{"badRequest": {"message": "Invalid input for field/attribute quota_set. Value: {u'foo': 20, u'force': True}. Additional properties are not allowed (u'foo' was unexpected)", "code": 400}}

Because the jsonschema prevents that:

https://github.com/openstack/nova/blob/master/nova/api/openstack/compute/schemas/quota_sets.py#L28

However, you can create a nova.quotas table entry with an invalid projectid:

http://paste.openstack.org/show/475511/

So we need to do something about that. I see two options:

1. quota-sets.update should only attempt to update quota on existing resources - if objects.Quotas.update_limit fails with a not found, we return 404 on the response. We don't try to create quotas automatically if they don't exist. However, as part of this we'd need to add a new quotas-sets.create API since update is handling both create and update today. This would require a microversion change since it's not backward compatible.

2. We could change quota-sets.update to try and update first and if we get a 404, then we validate the projectid against the identity service before calling objects.Quotas.create_limit. That would at least narrow the performance impact of validating the projectid against keystone in the request.

[Matt Riedemann (mriedem)]

Also, confirmed that you can pass a random uuid to nova quota-show and it will just return default quotas:

http://paste.openstack.org/show/475512/

Which is misleading if that project doesn't exist, but if you try updating it with the current behavior it will create the quotas entry in the db as detailed in comment 24.

[Abhilash Goyal (abhilash-goyal)]

I think this blueprint covers the same https://blueprints.launchpad.net/nova/+spec/validate-project-with-keystone. Kindly provide your feedback.

[Markus Zoeller (markus_z) (mzoeller)]

see comments #26 + #22

This wishlist bug has been open a year without any activity. I'm going to move it to "Opinion / Wishlist", which is an easily-obtainable queue of older requests that have come on. This bug can be reopened (set back to "New") if someone decides to work on this.

[Matt Riedemann (mriedem)]

This is being worked under this blueprint in the Pike release:

https://blueprints.launchpad.net/nova/+spec/validate-project-with-keystone

And has been fixed with this change in pike for the os-quota-sets API:

https://review.openstack.org/#/c/435010/