From 2b63f8796efb5cc177a15b0f4f4535a8f051212a Mon Sep 17 00:00:00 2001 From: Wayne Werner Date: Fri, 25 Jan 2019 15:08:32 -0600 Subject: [PATCH 1/2] Make debian_ip error messages handle non-text vals Right now it fails, badly, when an invalid value was passed, obscuring the real problem(s). This makes it str-ify the values first. Signed-off-by: Wayne Werner --- salt/modules/debian_ip.py | 4 ++-- tests/unit/modules/test_debian_ip.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/salt/modules/debian_ip.py b/salt/modules/debian_ip.py index b92a5d31f33d..8fbe9fb06ddb 100644 --- a/salt/modules/debian_ip.py +++ b/salt/modules/debian_ip.py @@ -158,7 +158,7 @@ def _error_msg_iface(iface, option, expected): a list of expected values. ''' msg = 'Invalid option -- Interface: {0}, Option: {1}, Expected: [{2}]' - return msg.format(iface, option, '|'.join(expected)) + return msg.format(iface, option, '|'.join(str(e) for e in expected)) def _error_msg_routes(iface, option, expected): @@ -181,7 +181,7 @@ def _error_msg_network(option, expected): a list of expected values. ''' msg = 'Invalid network setting -- Setting: {0}, Expected: [{1}]' - return msg.format(option, '|'.join(expected)) + return msg.format(option, '|'.join(str(e) for e in expected)) def _log_default_network(opt, value): diff --git a/tests/unit/modules/test_debian_ip.py b/tests/unit/modules/test_debian_ip.py index 1b9eb7857147..bb648034b1eb 100644 --- a/tests/unit/modules/test_debian_ip.py +++ b/tests/unit/modules/test_debian_ip.py @@ -814,6 +814,18 @@ def test_build_bond(self): 'pkg.install': mock}): self.assertEqual(debian_ip.build_bond('bond0'), '') + def test_error_message_iface_should_process_non_str_expected(self): + values = [1, True, False, 'no-kaboom'] + iface = 'ethtest' + option = 'test' + msg = debian_ip._error_msg_iface(iface, option, values) + self.assertTrue(msg.endswith('[1|True|False|no-kaboom]'), msg) + + def test_error_message_network_should_process_non_str_expected(self): + values = [1, True, False, 'no-kaboom'] + msg = debian_ip._error_msg_network('fnord', values) + self.assertTrue(msg.endswith('[1|True|False|no-kaboom]'), msg) + def test_build_bond_exception(self): ''' Test if it create a bond script in /etc/modprobe.d with the passed From 28f24654c7eb0ca4ce6dbe1f9830387372bbaa7a Mon Sep 17 00:00:00 2001 From: Wayne Werner Date: Fri, 25 Jan 2019 15:35:54 -0600 Subject: [PATCH 2/2] Make rh_ip error messages handle non-text values Previously, like the debian module, this would blow up with invalid values. Now it shows the proper error message. It looks like the incorrect behavior was introduced in the commit on Wed Apr 4 18:31:00 2012 -0600 - 45aaf46896c6a82d1da39fdb02f65540165688f0 Signed-off-by: Wayne Werner --- salt/modules/rh_ip.py | 4 ++-- tests/unit/modules/test_rh_ip.py | 12 ++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/salt/modules/rh_ip.py b/salt/modules/rh_ip.py index 26a6347eb2a3..e54069d9911b 100644 --- a/salt/modules/rh_ip.py +++ b/salt/modules/rh_ip.py @@ -81,7 +81,7 @@ def _error_msg_iface(iface, option, expected): a list of expected values. ''' msg = 'Invalid option -- Interface: {0}, Option: {1}, Expected: [{2}]' - return msg.format(iface, option, '|'.join(expected)) + return msg.format(iface, option, '|'.join(str(e) for e in expected)) def _error_msg_routes(iface, option, expected): @@ -104,7 +104,7 @@ def _error_msg_network(option, expected): a list of expected values. ''' msg = 'Invalid network setting -- Setting: {0}, Expected: [{1}]' - return msg.format(option, '|'.join(expected)) + return msg.format(option, '|'.join(str(e) for e in expected)) def _log_default_network(opt, value): diff --git a/tests/unit/modules/test_rh_ip.py b/tests/unit/modules/test_rh_ip.py index 78694de51c19..8664c067e7d3 100644 --- a/tests/unit/modules/test_rh_ip.py +++ b/tests/unit/modules/test_rh_ip.py @@ -32,6 +32,18 @@ class RhipTestCase(TestCase, LoaderModuleMockMixin): def setup_loader_modules(self): return {rh_ip: {}} + def test_error_message_iface_should_process_non_str_expected(self): + values = [1, True, False, 'no-kaboom'] + iface = 'ethtest' + option = 'test' + msg = rh_ip._error_msg_iface(iface, option, values) + self.assertTrue(msg.endswith('[1|True|False|no-kaboom]'), msg) + + def test_error_message_network_should_process_non_str_expected(self): + values = [1, True, False, 'no-kaboom'] + msg = rh_ip._error_msg_network('fnord', values) + self.assertTrue(msg.endswith('[1|True|False|no-kaboom]'), msg) + def test_build_bond(self): ''' Test to create a bond script in /etc/modprobe.d with the passed