Cinder

quota-update should error out if input provided is non-existent tenant id

Bug #1307491 reported by Tushar Bankar
48
This bug affects 13 people
Affects Status Importance Assigned to Milestone
Cinder
In Progress
Low
Rajat Dhasmana

Bug Description

cinder quota-update wrong-tenant --gigabytes 2

cinder returns 200 (OK).

It is expected to return 404 if tenant does not exists.

Tushar Bankar (tushar-bankar)
Changed in cinder:
assignee: nobody → Tushar Bankar (tushar-bankar)
Tushar Bankar (tushar-bankar)
tags: added: izeltech
Eric Harney (eharney)
tags: removed: izeltech
Mike Perez (thingee)
Changed in cinder:
status: New → Triaged
Harshada Mangesh Kakad (harshada-kakad)
Changed in cinder:
assignee: Tushar Bankar (tushar-bankar) → Harshada Mangesh Kakad (harshada-kakad)
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
status: Triaged → In Progress
Huang Zhiteng (zhiteng-huang)
summary: - quota-update should error out if input provided is wrong tenant id
+ quota-update should error out if input provided is non-existent tenant
+ id
description: updated
Revision history for this message
Huang Zhiteng (zhiteng-huang) wrote :

I'm not a HTTP expert, but I think 404 response is for HTTP GET request. Quota update is a POST request, I am not sure 404 is 'expected' result of non-existent tenant.

Revision history for this message
Mike Perez (thingee) wrote :

I think we return 404 for reasons like this already.

Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

I think this came up before, and there is no way to validate the tenant... e.g. do a quota update for a tenant that currently has default quota (i.e. no tenant specific db entry). Cinder would have to call keystone to validate the tenant, which (a) last time this came up a suitable keystone call was not found (b) requires a blocking call to keystone in our API code to get the behaviour above, which is something we try to avoid

Revision history for this message
Duncan Thomas (duncan-thomas) wrote :

More useful comments added to the review

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by Duncan Thomas (<email address hidden>) on branch: master
Review: https://review.openstack.org/95464
Reason: Abandoning change: No update for more than 14 days

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to cinder (master)

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

Changed in cinder:
assignee: Harshada Mangesh Kakad (harshada-kakad) → Anton Arefiev (aarefiev)
Jay Bryant (jsbryant)
Changed in cinder:
importance: Undecided → Medium
Mike Perez (thingee)
Changed in cinder:
milestone: none → kilo-2
Mike Perez (thingee)
Changed in cinder:
milestone: kilo-2 → kilo-3
Revision history for this message
Mike Perez (thingee) wrote :

Retargeting this. Duncan has objections and we're out of time in Kilo.

Changed in cinder:
milestone: kilo-3 → liberty-1
Revision history for this message
Vilobh Meshram (vilobhmm) wrote :

Anton Arefiev : Are you actively working on this Bug if not I can take it up and fix it.

Revision history for this message
Anton Arefiev (aarefiev) wrote :

Vilobh Meshram : my related patch https://review.openstack.org/#/c/143645/ and Rushi's https://review.openstack.org/#/c/139610/

Mike Perez (thingee)
Changed in cinder:
milestone: liberty-1 → liberty-2
OpenStack Infra (hudson-openstack)
Changed in cinder:
assignee: Anton Arefiev (aarefiev) → Duncan Thomas (duncan-thomas)
OpenStack Infra (hudson-openstack)
Changed in cinder:
assignee: Duncan Thomas (duncan-thomas) → Anton Arefiev (aarefiev)
OpenStack Infra (hudson-openstack)
Changed in cinder:
assignee: Anton Arefiev (aarefiev) → John Griffith (john-griffith)
Revision history for this message
stasis (rafael-galarza) wrote :

This bug also seems to apply to cinder quota-defaults

Mike Perez (thingee)
Changed in cinder:
milestone: liberty-2 → liberty-3
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on cinder (master)

Change abandoned by Anton Arefiev (<email address hidden>) on branch: master
Review: https://review.openstack.org/143645
Reason: More general approach is needed here

John Griffith (john-griffith)
Changed in cinder:
milestone: liberty-3 → none
assignee: John Griffith (john-griffith) → nobody
importance: Medium → Low
Robin Naundorf (senk)
Changed in cinder:
status: In Progress → Confirmed
haobing1 (haobing1)
Changed in cinder:
assignee: nobody → haobing1 (haobing1)
Revision history for this message
Sean McGinnis (sean-mcginnis) wrote : Owner Expired

Unassigning due to no activity.

Changed in cinder:
assignee: haobing1 (haobing1) → nobody
Xu Ao (xuao)
Changed in cinder:
assignee: nobody → Xu Ao (xuao)
Revision history for this message
Sean McGinnis (sean-mcginnis) wrote : Bug Assignee Expired

Unassigning due to no activity for > 6 months.

Changed in cinder:
assignee: Xu Ao (xuao) → nobody
Tejaswini Grandhe (grantejaswini)
Changed in cinder:
assignee: nobody → Tejaswini Grandhe (grantejaswini)
Tejaswini Grandhe (grantejaswini)
Changed in cinder:
assignee: Tejaswini Grandhe (grantejaswini) → nobody
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

Need to check on whether (a) there's a suitable keystone API call for this, and if so, (b) are we willing to block on calling keystone while processing a user request (that was Duncan's objection back in 2014).

We could at least make it partially correct by making sure we're being passed a UUID.

In any case, whoever picks up this bug should please discuss it at the cinder weekly meeting, or if you can't make the meeting, on the mailing list.

tags: added: low-hanging-fruit
Revision history for this message
Brian Rosmaita (brian-rosmaita) wrote :

BTW, about my previous comment -- would need to verify that a project ID must be a UUID. It wasn't always the case.

Revision history for this message
Walt Boring (walter-boring) wrote :

Currently the cinder client allows you pass in anything as a tenant ID

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

袁周立 (ma-yuan)
description: updated
袁周立 (ma-yuan)
description: updated
Revision history for this message
Gorka Eguileor (gorka) wrote :

There is a Keystone API to check if a project exists. I confirmed it while writing the custom default type spec.

So once Rajat implements that spec we will have a method that can validate the existence of a project, and we can reuse it to fix this bug.

Revision history for this message
Gorka Eguileor (gorka) wrote :

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

Changed in cinder:
assignee: nobody → Rajat Dhasmana (whoami-rajat)
status: Confirmed → In Progress
Revision history for this message
Chengxu Lan (chelsy-lan) wrote :

Hi Rajat, if you are still working on this bug? I'm an Outreachy applicant and would like to contribute to the Cinder.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.