From fc9163fce9c4e9deca67a6444db8f913bec15ec3 Mon Sep 17 00:00:00 2001 From: Michael Johnson Date: Wed, 29 May 2019 14:35:57 -0700 Subject: [PATCH] Fix member API handling of None/null updates The current member API does not properly handle clearing/reseting values on update. Some integer only fields, such as weight, will accept null, but will store the value as "None". These will will cause failures updating the amphora configuration. This patch corrects this to appropriately handle None/null updates to the member parameters. Change-Id: I41038b1d8d882efa19d991c07dca47f06dcbb5ca Story: 2005374 Task: 33523 --- octavia/api/v2/controllers/member.py | 18 +++++++++++++++++ octavia/api/v2/types/member.py | 6 ++++-- octavia/common/constants.py | 1 + .../tests/functional/api/v2/test_member.py | 20 +++++++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/octavia/api/v2/controllers/member.py b/octavia/api/v2/controllers/member.py index a70ac4850f..bd73fc7f8f 100644 --- a/octavia/api/v2/controllers/member.py +++ b/octavia/api/v2/controllers/member.py @@ -206,6 +206,22 @@ class MemberController(base.BaseController): return db_member + def _set_default_on_none(self, member): + """Reset settings to their default values if None/null was passed in + + A None/null value can be passed in to clear a value. PUT values + that were not provided by the user have a type of wtypes.UnsetType. + If the user is attempting to clear values, they should either + be set to None (for example in the name field) or they should be + reset to their default values. + This method is intended to handle those values that need to be set + back to a default value. + """ + if member.backup is None: + member.backup = False + if member.weight is None: + member.weight = constants.DEFAULT_WEIGHT + @wsme_pecan.wsexpose(member_types.MemberRootResponse, wtypes.text, body=member_types.MemberRootPUT, status_code=200) @@ -225,6 +241,8 @@ class MemberController(base.BaseController): self._validate_pool_id(id, db_member.pool_id) + self._set_default_on_none(member) + # Load the driver early as it also provides validation driver = driver_factory.get_driver(provider) diff --git a/octavia/api/v2/types/member.py b/octavia/api/v2/types/member.py index cb2159a541..246a892ab1 100644 --- a/octavia/api/v2/types/member.py +++ b/octavia/api/v2/types/member.py @@ -74,7 +74,8 @@ class MemberPOST(BaseMemberType): minimum=constants.MIN_PORT_NUMBER, maximum=constants.MAX_PORT_NUMBER), mandatory=True) weight = wtypes.wsattr(wtypes.IntegerType( - minimum=constants.MIN_WEIGHT, maximum=constants.MAX_WEIGHT), default=1) + minimum=constants.MIN_WEIGHT, maximum=constants.MAX_WEIGHT), + default=constants.DEFAULT_WEIGHT) backup = wtypes.wsattr(bool, default=False) subnet_id = wtypes.wsattr(wtypes.UuidType()) # TODO(johnsom) Remove after deprecation (R series) @@ -120,7 +121,8 @@ class MemberSingleCreate(BaseMemberType): minimum=constants.MIN_PORT_NUMBER, maximum=constants.MAX_PORT_NUMBER), mandatory=True) weight = wtypes.wsattr(wtypes.IntegerType( - minimum=constants.MIN_WEIGHT, maximum=constants.MAX_WEIGHT), default=1) + minimum=constants.MIN_WEIGHT, maximum=constants.MAX_WEIGHT), + default=constants.DEFAULT_WEIGHT) backup = wtypes.wsattr(bool, default=False) subnet_id = wtypes.wsattr(wtypes.UuidType()) monitor_port = wtypes.wsattr(wtypes.IntegerType( diff --git a/octavia/common/constants.py b/octavia/common/constants.py index f86480496e..9641a235eb 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -209,6 +209,7 @@ MAX_PORT_NUMBER = 65535 DEFAULT_CONNECTION_LIMIT = -1 MIN_CONNECTION_LIMIT = -1 +DEFAULT_WEIGHT = 1 MIN_WEIGHT = 0 MAX_WEIGHT = 256 diff --git a/octavia/tests/functional/api/v2/test_member.py b/octavia/tests/functional/api/v2/test_member.py index 2124fd6eb8..a34d9d7971 100644 --- a/octavia/tests/functional/api/v2/test_member.py +++ b/octavia/tests/functional/api/v2/test_member.py @@ -1074,6 +1074,26 @@ class TestMember(base.BaseAPITest): self.assertIn('Provider \'bad_driver\' reports error: broken', response.json.get('faultstring')) + def test_update_unset_defaults(self): + old_name = "name1" + api_member = self.create_member( + self.pool_with_listener_id, '192.0.2.1', 80, + name=old_name, backup=True, monitor_address='192.0.2.2', + monitor_port=8888, weight=10).get(self.root_tag) + self.set_lb_status(self.lb_id) + unset_params = {'name': None, 'backup': None, 'monitor_address': None, + 'monitor_port': None, 'weight': None} + member_path = self.member_path_listener.format( + member_id=api_member.get('id')) + response = self.put(member_path, self._build_body(unset_params)) + response = response.json.get(self.root_tag) + + self.assertFalse(response['backup']) + self.assertIsNone(response['monitor_address']) + self.assertIsNone(response['monitor_port']) + self.assertEqual('', response['name']) + self.assertEqual(constants.DEFAULT_WEIGHT, response['weight']) + def test_delete(self): api_member = self.create_member( self.pool_with_listener_id, '192.0.2.1', 80).get(self.root_tag)