diff --git a/ironic/api/controllers/v1/chassis.py b/ironic/api/controllers/v1/chassis.py index c6f42a7b1a..9059b7420f 100644 --- a/ironic/api/controllers/v1/chassis.py +++ b/ironic/api/controllers/v1/chassis.py @@ -28,6 +28,7 @@ from ironic.api.controllers.v1 import types from ironic.api.controllers.v1 import utils as api_utils from ironic.api import expose from ironic.common import exception +from ironic.common.i18n import _ from ironic import objects @@ -147,6 +148,8 @@ class ChassisController(rest.RestController): 'detail': ['GET'], } + invalid_sort_key_list = ['extra'] + def _get_chassis_collection(self, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): limit = api_utils.validate_limit(limit) @@ -155,6 +158,12 @@ class ChassisController(rest.RestController): if marker: marker_obj = objects.Chassis.get_by_uuid(pecan.request.context, marker) + + if sort_key in self.invalid_sort_key_list: + raise exception.InvalidParameterValue(_( + "The sort_key value %(key)s is an invalid field for sorting") + % {'key': sort_key}) + chassis = objects.Chassis.list(pecan.request.context, limit, marker_obj, sort_key=sort_key, sort_dir=sort_dir) diff --git a/ironic/api/controllers/v1/node.py b/ironic/api/controllers/v1/node.py index 26beaaa3fa..d16cfbc434 100644 --- a/ironic/api/controllers/v1/node.py +++ b/ironic/api/controllers/v1/node.py @@ -786,6 +786,9 @@ class NodesController(rest.RestController): 'validate': ['GET'], } + invalid_sort_key_list = ['properties', 'driver_info', 'extra', + 'instance_info', 'driver_internal_info'] + def _get_nodes_collection(self, chassis_uuid, instance_uuid, associated, maintenance, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): @@ -800,6 +803,12 @@ class NodesController(rest.RestController): if marker: marker_obj = objects.Node.get_by_uuid(pecan.request.context, marker) + + if sort_key in self.invalid_sort_key_list: + raise exception.InvalidParameterValue(_( + "The sort_key value %(key)s is an invalid field for sorting") + % {'key': sort_key}) + if instance_uuid: nodes = self._get_nodes_by_instance(instance_uuid) else: diff --git a/ironic/api/controllers/v1/port.py b/ironic/api/controllers/v1/port.py index 9ceaafb1f1..f929a8265a 100644 --- a/ironic/api/controllers/v1/port.py +++ b/ironic/api/controllers/v1/port.py @@ -176,6 +176,8 @@ class PortsController(rest.RestController): 'detail': ['GET'], } + invalid_sort_key_list = ['extra'] + def _get_ports_collection(self, node_ident, address, marker, limit, sort_key, sort_dir, expand=False, resource_url=None): @@ -191,6 +193,11 @@ class PortsController(rest.RestController): marker_obj = objects.Port.get_by_uuid(pecan.request.context, marker) + if sort_key in self.invalid_sort_key_list: + raise exception.InvalidParameterValue(_( + "The sort_key value %(key)s is an invalid field for sorting" + ) % {'key': sort_key}) + if node_ident: # FIXME(comstud): Since all we need is the node ID, we can # make this more efficient by only querying diff --git a/ironic/tests/api/v1/test_chassis.py b/ironic/tests/api/v1/test_chassis.py index 70a9a6d7aa..140ab8bbf3 100644 --- a/ironic/tests/api/v1/test_chassis.py +++ b/ironic/tests/api/v1/test_chassis.py @@ -128,12 +128,13 @@ class TestListChassis(api_base.FunctionalTest): self.assertEqual(sorted(ch_list), uuids) def test_sort_key_invalid(self): - invalid_key = 'foo' - response = self.get_json('/chassis?sort_key=%s' % invalid_key, - expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertIn(invalid_key, response.json['error_message']) + invalid_keys_list = ['foo', 'extra'] + for invalid_key in invalid_keys_list: + response = self.get_json('/chassis?sort_key=%s' % invalid_key, + expect_errors=True) + self.assertEqual(400, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertIn(invalid_key, response.json['error_message']) def test_nodes_subresource_link(self): chassis = obj_utils.create_test_chassis(self.context) diff --git a/ironic/tests/api/v1/test_nodes.py b/ironic/tests/api/v1/test_nodes.py index d3893df9dc..58e0d4cbfd 100644 --- a/ironic/tests/api/v1/test_nodes.py +++ b/ironic/tests/api/v1/test_nodes.py @@ -285,12 +285,14 @@ class TestListNodes(test_api_base.FunctionalTest): self.assertEqual(sorted(nodes), uuids) def test_sort_key_invalid(self): - invalid_key = 'foo' - response = self.get_json('/nodes?sort_key=%s' % invalid_key, - expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertIn(invalid_key, response.json['error_message']) + invalid_keys_list = ['foo', 'properties', 'driver_info', 'extra', + 'instance_info', 'driver_internal_info'] + for invalid_key in invalid_keys_list: + response = self.get_json('/nodes?sort_key=%s' % invalid_key, + expect_errors=True) + self.assertEqual(400, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertIn(invalid_key, response.json['error_message']) def test_ports_subresource_link(self): node = obj_utils.create_test_node(self.context) diff --git a/ironic/tests/api/v1/test_ports.py b/ironic/tests/api/v1/test_ports.py index 4111b8a7b4..b08c0327f8 100644 --- a/ironic/tests/api/v1/test_ports.py +++ b/ironic/tests/api/v1/test_ports.py @@ -196,12 +196,13 @@ class TestListPorts(api_base.FunctionalTest): self.assertEqual(sorted(ports), uuids) def test_sort_key_invalid(self): - invalid_key = 'foo' - response = self.get_json('/ports?sort_key=%s' % invalid_key, - expect_errors=True) - self.assertEqual(400, response.status_int) - self.assertEqual('application/json', response.content_type) - self.assertIn(invalid_key, response.json['error_message']) + invalid_keys_list = ['foo', 'extra'] + for invalid_key in invalid_keys_list: + response = self.get_json('/ports?sort_key=%s' % invalid_key, + expect_errors=True) + self.assertEqual(400, response.status_int) + self.assertEqual('application/json', response.content_type) + self.assertIn(invalid_key, response.json['error_message']) @mock.patch.object(api_utils, 'get_rpc_node') def test_get_all_by_node_name_ok(self, mock_get_rpc_node):