Return HTTP 200 on list for invalid status
The server listing API raises a 500 error if you pass an invalid status filter for admin user. In the case of a non-admin user, it simply returns an empty list. In the case of an admin user, it fetches extended server attributes, so a condition was added to get extended server attributes only when servers list is not empty. This change simply removes the cause of the 500 exception. A subsequent patch with a microversion bump will modify the behavior so that a 400 Bad Request will be raised for an invalid status, for both admin and non-admin alike. Co-Authored-By: Dinesh Bhor <dinesh.bhor@nttdata.com> Closes-Bug: #1579706 Change-Id: I10bde78f0a9ac59b8646d58f62fa5056f989f54f
This commit is contained in:
parent
222a88a54c
commit
ee4d69e28d
@ -91,18 +91,21 @@ class ExtendedServerAttributesController(wsgi.Controller):
|
|||||||
authorize_host_status = True
|
authorize_host_status = True
|
||||||
if authorize_extend or authorize_host_status:
|
if authorize_extend or authorize_host_status:
|
||||||
servers = list(resp_obj.obj['servers'])
|
servers = list(resp_obj.obj['servers'])
|
||||||
instances = req.get_db_instances()
|
# NOTE(dinesh-bhor): Skipping fetching of instances from cache as
|
||||||
# Instances is guaranteed to be in the cache due to
|
# servers list can be empty if invalid status is provided to the
|
||||||
# the core API adding it in its 'detail' method.
|
# core API 'detail' method.
|
||||||
if authorize_host_status:
|
if servers:
|
||||||
host_statuses = self.compute_api.get_instances_host_statuses(
|
instances = req.get_db_instances()
|
||||||
instances.values())
|
|
||||||
for server in servers:
|
|
||||||
if authorize_extend:
|
|
||||||
instance = instances[server['id']]
|
|
||||||
self._extend_server(context, server, instance, req)
|
|
||||||
if authorize_host_status:
|
if authorize_host_status:
|
||||||
server['host_status'] = host_statuses[server['id']]
|
host_statuses = (
|
||||||
|
self.compute_api.get_instances_host_statuses(
|
||||||
|
instances.values()))
|
||||||
|
for server in servers:
|
||||||
|
if authorize_extend:
|
||||||
|
instance = instances[server['id']]
|
||||||
|
self._extend_server(context, server, instance, req)
|
||||||
|
if authorize_host_status:
|
||||||
|
server['host_status'] = host_statuses[server['id']]
|
||||||
|
|
||||||
|
|
||||||
class ExtendedServerAttributes(extensions.V21APIExtensionBase):
|
class ExtendedServerAttributes(extensions.V21APIExtensionBase):
|
||||||
|
@ -13,6 +13,7 @@
|
|||||||
# License for the specific language governing permissions and limitations
|
# License for the specific language governing permissions and limitations
|
||||||
# under the License.
|
# under the License.
|
||||||
|
|
||||||
|
import mock
|
||||||
from oslo_config import cfg
|
from oslo_config import cfg
|
||||||
from oslo_serialization import jsonutils
|
from oslo_serialization import jsonutils
|
||||||
|
|
||||||
@ -129,6 +130,17 @@ class ExtendedServerAttributesTestV21(test.TestCase):
|
|||||||
node='node-%s' % (i + 1),
|
node='node-%s' % (i + 1),
|
||||||
instance_name=NAME_FMT % (i + 1))
|
instance_name=NAME_FMT % (i + 1))
|
||||||
|
|
||||||
|
@mock.patch.object(compute.api.API, 'get_all')
|
||||||
|
def test_detail_empty_instance_list_invalid_status(self,
|
||||||
|
mock_get_all_method):
|
||||||
|
mock_get_all_method.return_value = objects.InstanceList(objects=[])
|
||||||
|
|
||||||
|
url = "%s%s" % (self.fake_url, '/servers/detail?status=invalid_status')
|
||||||
|
res = self._make_request(url)
|
||||||
|
# check status code 200 with empty instance list
|
||||||
|
self.assertEqual(200, res.status_int)
|
||||||
|
self.assertEqual(0, len(self._get_servers(res.body)))
|
||||||
|
|
||||||
def test_no_instance_passthrough_404(self):
|
def test_no_instance_passthrough_404(self):
|
||||||
|
|
||||||
def fake_compute_get(*args, **kwargs):
|
def fake_compute_get(*args, **kwargs):
|
||||||
|
@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Fixed bug #1579706: "Listing nova instances with invalid status raises 500
|
||||||
|
InternalServerError for admin user". Now passing an invalid status as a
|
||||||
|
filter will return an empty list. A subsequent patch will then correct this
|
||||||
|
to raise a 400 Bad Request when an invalid status is received.
|
Loading…
Reference in New Issue
Block a user