From e5b5ff5d5613b4331dd279e709ecf518e3b0170c Mon Sep 17 00:00:00 2001 From: Alfredo Moralejo Date: Wed, 14 May 2025 09:20:25 +0200 Subject: [PATCH] Return HTTP code 400 when creating an audit with wrong parameters Currently, when trying to create an audit which misses a mandatory parameter watcher returns error 500 instead of 400 which is the documented error in the API [1] and the appropiate error code for malformed requests. This patch catch parameters validation errors according to the json schema for each strategy and returns error 400. It also fixes the unit test to validate the expected behavior. [1] https://docs.openstack.org/api-ref/resource-optimization/#audits Closes-Bug: #2110538 Change-Id: I23232b3b54421839bb01d54386d4e7b244f4e2a0 (cherry picked from commit 4629402f384601778c9583807518214764da2804) Signed-off-by: Alfredo Moralejo --- ...ror-400-on-bad-parameters-bb964e4f5cadc15c.yaml | 7 +++++++ watcher/api/controllers/v1/audit.py | 9 +++++++-- watcher/tests/api/v1/test_audits.py | 14 +++++--------- 3 files changed, 19 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/return-error-400-on-bad-parameters-bb964e4f5cadc15c.yaml diff --git a/releasenotes/notes/return-error-400-on-bad-parameters-bb964e4f5cadc15c.yaml b/releasenotes/notes/return-error-400-on-bad-parameters-bb964e4f5cadc15c.yaml new file mode 100644 index 000000000..9c920f2ed --- /dev/null +++ b/releasenotes/notes/return-error-400-on-bad-parameters-bb964e4f5cadc15c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + `Bug #2110538 `_: + Corrected the HTTP error code returned when watcher users try to create + audits with invalid parameters. The API now correctly returns a 400 Bad + Request error. diff --git a/watcher/api/controllers/v1/audit.py b/watcher/api/controllers/v1/audit.py index 70c825e27..366e7c628 100644 --- a/watcher/api/controllers/v1/audit.py +++ b/watcher/api/controllers/v1/audit.py @@ -33,6 +33,7 @@ import datetime from dateutil import tz from http import HTTPStatus +import jsonschema from oslo_log import log from oslo_utils import timeutils import pecan @@ -627,8 +628,12 @@ class AuditsController(rest.RestController): if schema: # validate input parameter with default value feedback no_schema = False - utils.StrictDefaultValidatingDraft4Validator(schema).validate( - audit.parameters) + try: + utils.StrictDefaultValidatingDraft4Validator( + schema).validate(audit.parameters) + except jsonschema.exceptions.ValidationError as e: + raise exception.Invalid( + _('Invalid parameters for strategy: %s') % e) if no_schema and audit.parameters: raise exception.Invalid(_('Specify parameters but no predefined ' diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index adcf46afc..c536be871 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -843,16 +843,12 @@ class TestPost(api_base.FunctionalTest): del audit_dict[k] response = self.post_json('/audits', audit_dict, expect_errors=True) - # (amoralej) This should return HTTPStatus.BAD_REQUEST, however this - # review is adding the test to show wrong code is returned. I will - # switch this to be HTTPStatus.BAD_REQUEST in the fixing review. - self.assertEqual(HTTPStatus.INTERNAL_SERVER_ERROR, response.status_int) + self.assertEqual(HTTPStatus.BAD_REQUEST, response.status_int) self.assertEqual("application/json", response.content_type) - # (amoralej) uncomment with the fix - # expected_error_msg = ( - # "Invalid parameters for strategy: 'fake1' is a required property") - # self.assertTrue(response.json['error_message']) - # self.assertIn(expected_error_msg, response.json['error_message']) + expected_error_msg = ( + "Invalid parameters for strategy: 'fake1' is a required property") + self.assertTrue(response.json['error_message']) + self.assertIn(expected_error_msg, response.json['error_message']) assert not mock_trigger_audit.called def prepare_audit_template_strategy_with_parameter(self):