From 9a1dc718b19f3d4c73be213296f37e3069d871b1 Mon Sep 17 00:00:00 2001 From: Kaifeng Wang Date: Tue, 12 Jun 2018 20:47:30 +0800 Subject: [PATCH] Validating fault value when querying with fault field This patch adds checking to fault field when querying node with it. Bad request will be raised for invalid fault value. Change-Id: Ifc2de52cbe897a7aab4910e1c00df1ae933bb9d8 Story: #1596107 Task: #10469 --- ironic/api/controllers/v1/utils.py | 8 ++++++++ ironic/common/faults.py | 2 ++ ironic/tests/unit/api/controllers/v1/test_node.py | 13 ++++++++++++- 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/ironic/api/controllers/v1/utils.py b/ironic/api/controllers/v1/utils.py index e97cadb75c..c740f5d4c0 100644 --- a/ironic/api/controllers/v1/utils.py +++ b/ironic/api/controllers/v1/utils.py @@ -29,6 +29,7 @@ import wsme from ironic.api.controllers.v1 import versions from ironic.common import exception +from ironic.common import faults from ironic.common.i18n import _ from ironic.common import states from ironic.common import utils @@ -491,6 +492,13 @@ def check_allow_filter_by_fault(fault): "should be %(base)s.%(opr)s") % {'base': versions.BASE_VERSION, 'opr': versions.MINOR_42_FAULT}) + if fault is not None and fault not in faults.VALID_FAULTS: + msg = (_('Unrecognized fault "%(fault)s" is specified, allowed faults ' + 'are %(valid_faults)s') % + {'fault': fault, 'valid_faults': faults.VALID_FAULTS}) + raise wsme.exc.ClientSideError( + msg, status_code=http_client.BAD_REQUEST) + def initial_node_provision_state(): """Return node state to use by default when creating new nodes. diff --git a/ironic/common/faults.py b/ironic/common/faults.py index dd7ab955c7..b794950779 100644 --- a/ironic/common/faults.py +++ b/ironic/common/faults.py @@ -23,3 +23,5 @@ CLEAN_FAILURE = 'clean failure' RESCUE_ABORT_FAILURE = 'rescue abort failure' """ Node is moved to maintenance due to failure of cleaning up during rescue abort. """ + +VALID_FAULTS = (POWER_FAILURE, CLEAN_FAILURE, RESCUE_ABORT_FAILURE) diff --git a/ironic/tests/unit/api/controllers/v1/test_node.py b/ironic/tests/unit/api/controllers/v1/test_node.py index 2de2a603fa..6f1a3e6db5 100644 --- a/ironic/tests/unit/api/controllers/v1/test_node.py +++ b/ironic/tests/unit/api/controllers/v1/test_node.py @@ -1313,8 +1313,19 @@ class TestListNodes(test_api_base.BaseApiTest): self.assertIn(node2.uuid, uuids) self.assertNotIn(node1.uuid, uuids) + def test_get_nodes_by_fault_with_invalid_fault(self): + for url in ('/nodes?fault=somefake', + '/nodes/detail?fault=somefake'): + response = self.get_json( + url, headers={api_base.Version.string: "1.42"}, + expect_errors=True) + self.assertEqual('application/json', response.content_type) + self.assertEqual(http_client.BAD_REQUEST, response.status_code) + self.assertTrue(response.json['error_message']) + def test_get_nodes_by_fault_not_allowed(self): - for url in ('/nodes?fault=test', '/nodes/detail?fault=test'): + for url in ('/nodes?fault=power failure', + '/nodes/detail?fault=power failure'): response = self.get_json( url, headers={api_base.Version.string: "1.41"}, expect_errors=True)