diff --git a/nova/api/openstack/compute/extended_server_attributes.py b/nova/api/openstack/compute/extended_server_attributes.py index 66061f2b9f38..f06bd9100e4d 100644 --- a/nova/api/openstack/compute/extended_server_attributes.py +++ b/nova/api/openstack/compute/extended_server_attributes.py @@ -89,18 +89,21 @@ class ExtendedServerAttributesController(wsgi.Controller): authorize_host_status = True if authorize_extend or authorize_host_status: servers = list(resp_obj.obj['servers']) - instances = req.get_db_instances() - # Instances is guaranteed to be in the cache due to - # the core API adding it in its 'detail' method. - if authorize_host_status: - 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) + # NOTE(dinesh-bhor): Skipping fetching of instances from cache as + # servers list can be empty if invalid status is provided to the + # core API 'detail' method. + if servers: + instances = req.get_db_instances() 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): diff --git a/nova/tests/unit/api/openstack/compute/test_extended_server_attributes.py b/nova/tests/unit/api/openstack/compute/test_extended_server_attributes.py index 834044fbcd86..b8da1eb04f77 100644 --- a/nova/tests/unit/api/openstack/compute/test_extended_server_attributes.py +++ b/nova/tests/unit/api/openstack/compute/test_extended_server_attributes.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import mock from oslo_config import cfg from oslo_serialization import jsonutils import webob @@ -130,6 +131,17 @@ class ExtendedServerAttributesTestV21(test.TestCase): node='node-%s' % (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 fake_compute_get(*args, **kwargs): diff --git a/releasenotes/notes/list-server-bad-status-fix-7db504b38c8d732f.yaml b/releasenotes/notes/list-server-bad-status-fix-7db504b38c8d732f.yaml new file mode 100644 index 000000000000..6a92c7c1fa01 --- /dev/null +++ b/releasenotes/notes/list-server-bad-status-fix-7db504b38c8d732f.yaml @@ -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.