From ab8a263674a918bf5a4f1c7ebf20594baad1d65a Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Thu, 30 May 2019 15:10:53 -0700 Subject: [PATCH] Fix flavor profile API handling of None updates The current flavor profile API does not properly handle clearing/reseting values on update. Some mandatory fields would return a database "cannot be Null" error. This patch raises the proper invalid option execption. Story: 2005374 Task: 33542 Change-Id: I5253c48871a8bb3bf91f82aa7791585cc4a6d529 --- octavia/api/v2/controllers/flavor_profiles.py | 26 ++++++++++++++----- octavia/common/constants.py | 2 ++ .../functional/api/v2/test_flavor_profiles.py | 21 ++++++++++++++- ...API-update-null-None-1b400962017a3d56.yaml | 6 +++++ 4 files changed, 47 insertions(+), 8 deletions(-) create mode 100644 releasenotes/notes/Fix-API-update-null-None-1b400962017a3d56.yaml diff --git a/octavia/api/v2/controllers/flavor_profiles.py b/octavia/api/v2/controllers/flavor_profiles.py index 5d5deea1cf..0de743283d 100644 --- a/octavia/api/v2/controllers/flavor_profiles.py +++ b/octavia/api/v2/controllers/flavor_profiles.py @@ -111,6 +111,24 @@ class FlavorProfileController(base.BaseController): db_flavor_profile, profile_types.FlavorProfileResponse) return profile_types.FlavorProfileRootResponse(flavorprofile=result) + def _validate_update_fp(self, context, id, flavorprofile): + if flavorprofile.name is None: + raise exceptions.InvalidOption(value=None, option=constants.NAME) + if flavorprofile.provider_name is None: + raise exceptions.InvalidOption(value=None, + option=constants.PROVIDER_NAME) + if flavorprofile.flavor_data is None: + raise exceptions.InvalidOption(value=None, + option=constants.FLAVOR_DATA) + + # Don't allow changes to the flavor_data or provider_name if it + # is in use. + if (not isinstance(flavorprofile.flavor_data, wtypes.UnsetType) or + not isinstance(flavorprofile.provider_name, wtypes.UnsetType)): + if self.repositories.flavor.count(context.session, + flavor_profile_id=id) > 0: + raise exceptions.ObjectInUse(object='Flavor profile', id=id) + @wsme_pecan.wsexpose(profile_types.FlavorProfileRootResponse, wtypes.text, status_code=200, body=profile_types.FlavorProfileRootPUT) @@ -121,13 +139,7 @@ class FlavorProfileController(base.BaseController): self._auth_validate_action(context, context.project_id, constants.RBAC_PUT) - # Don't allow changes to the flavor_data or provider_name if it - # is in use. - if (not isinstance(flavorprofile.flavor_data, wtypes.UnsetType) or - not isinstance(flavorprofile.provider_name, wtypes.UnsetType)): - if self.repositories.flavor.count(context.session, - flavor_profile_id=id) > 0: - raise exceptions.ObjectInUse(object='Flavor profile', id=id) + self._validate_update_fp(context, id, flavorprofile) if not isinstance(flavorprofile.flavor_data, wtypes.UnsetType): # Do a basic JSON validation on the metadata diff --git a/octavia/common/constants.py b/octavia/common/constants.py index f86480496e..f81eaf39e8 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -340,6 +340,8 @@ BYTES_IN = 'bytes_in' BYTES_OUT = 'bytes_out' REQUEST_ERRORS = 'request_errors' TOTAL_CONNECTIONS = 'total_connections' +NAME = 'name' +PROVIDER_NAME = 'provider_name' CERT_ROTATE_AMPHORA_FLOW = 'octavia-cert-rotate-amphora-flow' CREATE_AMPHORA_FLOW = 'octavia-create-amphora-flow' diff --git a/octavia/tests/functional/api/v2/test_flavor_profiles.py b/octavia/tests/functional/api/v2/test_flavor_profiles.py index 1ffb8b61d8..869d39ed9a 100644 --- a/octavia/tests/functional/api/v2/test_flavor_profiles.py +++ b/octavia/tests/functional/api/v2/test_flavor_profiles.py @@ -328,7 +328,7 @@ class TestFlavorProfiles(base.BaseAPITest): self.assertEqual('{"hello": "world"}', response.get(constants.FLAVOR_DATA)) - def test_update_none(self): + def test_update_nothing(self): fp = self.create_flavor_profile('test_profile', 'noop_driver', '{"x": "y"}') body = self._build_body({}) @@ -340,6 +340,25 @@ class TestFlavorProfiles(base.BaseAPITest): self.assertEqual('{"x": "y"}', response.get(constants.FLAVOR_DATA)) + def test_update_name_none(self): + self._test_update_param_none(constants.NAME) + + def test_update_provider_name_none(self): + self._test_update_param_none(constants.PROVIDER_NAME) + + def test_update_flavor_data_none(self): + self._test_update_param_none(constants.FLAVOR_DATA) + + def _test_update_param_none(self, param_name): + fp = self.create_flavor_profile('test_profile', 'noop_driver', + '{"x": "y"}') + expect_error_msg = ("None is not a valid option for %s" % + param_name) + body = self._build_body({param_name: None}) + response = self.put(self.FP_PATH.format(fp_id=fp.get('id')), body, + status=400) + self.assertEqual(expect_error_msg, response.json['faultstring']) + def test_update_no_flavor_data(self): fp = self.create_flavor_profile('test_profile', 'noop_driver', '{"x": "y"}') diff --git a/releasenotes/notes/Fix-API-update-null-None-1b400962017a3d56.yaml b/releasenotes/notes/Fix-API-update-null-None-1b400962017a3d56.yaml new file mode 100644 index 0000000000..b77e203f46 --- /dev/null +++ b/releasenotes/notes/Fix-API-update-null-None-1b400962017a3d56.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + Fixed the API handling of None (JSON null) on object update calls. The + API will now either clear the value from the field or will reset the value + of the field to the API default.