From 936c9b908e5be0d38c86bbc5ad0159f5bbb79b5b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Nov 2017 16:09:10 +0100 Subject: [PATCH 1/9] library/trivial: move code around Have the base types of ArgValidator classes closer together. --- library/network_connections.py | 80 +++++++++++++++++----------------- 1 file changed, 40 insertions(+), 40 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 789f118..b12ac40 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -517,46 +517,6 @@ class ArgValidatorBool(ArgValidator): pass raise ValidationError(name, 'must be an boolean but is "%s"' % (value)) -class ArgValidatorIP(ArgValidatorStr): - def __init__(self, name, family = None, required = False, default_value = None, plain_address = True): - ArgValidatorStr.__init__(self, name, required, default_value, None) - self.family = family - self.plain_address = plain_address - def _validate(self, value, name): - v = ArgValidatorStr._validate(self, value, name) - try: - addr, family = Util.parse_ip(v, self.family) - except: - raise ValidationError(name, 'value "%s" is not a valid IP%s address' % (value, Util.addr_family_to_v(self.family))) - if self.plain_address: - return addr - return { 'is_v4': family == socket.AF_INET, 'family': family, 'address': addr } - -class ArgValidatorMac(ArgValidatorStr): - def __init__(self, name, force_len = None, required = False, default_value = None): - ArgValidatorStr.__init__(self, name, required, default_value, None) - self.force_len = force_len - def _validate(self, value, name): - v = ArgValidatorStr._validate(self, value, name) - try: - addr = Util.mac_aton(v, self.force_len) - except MyError as e: - raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) - if not addr: - raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) - return Util.mac_ntoa(addr) - -class ArgValidatorIPAddr(ArgValidatorStr): - def __init__(self, name, family = None, required = False, default_value = None): - ArgValidatorStr.__init__(self, name, required, default_value, None) - self.family = family - def _validate(self, value, name): - v = ArgValidatorStr._validate(self, value, name) - try: - return Util.parse_address(v, self.family) - except: - raise ValidationError(name, 'value "%s" is not a valid IP%s address with prefix length' % (value, Util.addr_family_to_v(self.family))) - class ArgValidatorDict(ArgValidator): def __init__(self, name = None, required = False, nested = None, default_value = None, all_missing_during_validate = False): ArgValidator.__init__(self, name, required, default_value) @@ -617,6 +577,46 @@ class ArgValidatorList(ArgValidator): result.append(vv) return result +class ArgValidatorIP(ArgValidatorStr): + def __init__(self, name, family = None, required = False, default_value = None, plain_address = True): + ArgValidatorStr.__init__(self, name, required, default_value, None) + self.family = family + self.plain_address = plain_address + def _validate(self, value, name): + v = ArgValidatorStr._validate(self, value, name) + try: + addr, family = Util.parse_ip(v, self.family) + except: + raise ValidationError(name, 'value "%s" is not a valid IP%s address' % (value, Util.addr_family_to_v(self.family))) + if self.plain_address: + return addr + return { 'is_v4': family == socket.AF_INET, 'family': family, 'address': addr } + +class ArgValidatorMac(ArgValidatorStr): + def __init__(self, name, force_len = None, required = False, default_value = None): + ArgValidatorStr.__init__(self, name, required, default_value, None) + self.force_len = force_len + def _validate(self, value, name): + v = ArgValidatorStr._validate(self, value, name) + try: + addr = Util.mac_aton(v, self.force_len) + except MyError as e: + raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) + if not addr: + raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) + return Util.mac_ntoa(addr) + +class ArgValidatorIPAddr(ArgValidatorStr): + def __init__(self, name, family = None, required = False, default_value = None): + ArgValidatorStr.__init__(self, name, required, default_value, None) + self.family = family + def _validate(self, value, name): + v = ArgValidatorStr._validate(self, value, name) + try: + return Util.parse_address(v, self.family) + except: + raise ValidationError(name, 'value "%s" is not a valid IP%s address with prefix length' % (value, Util.addr_family_to_v(self.family))) + class ArgValidator_DictIP(ArgValidatorDict): def __init__(self): ArgValidatorDict.__init__(self, From 948fd7bf361eeb973fdcaff7a900dbc54f41472c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Nov 2017 16:22:36 +0100 Subject: [PATCH 2/9] library: derive ArgValidatorIPAddr from ArgValidator instead of ArgValidatorStr Next we will hack it up, to also support dictionary input arguments. --- library/network_connections.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index b12ac40..b8f1421 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -606,12 +606,16 @@ class ArgValidatorMac(ArgValidatorStr): raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) return Util.mac_ntoa(addr) -class ArgValidatorIPAddr(ArgValidatorStr): +class ArgValidatorIPAddr(ArgValidator): def __init__(self, name, family = None, required = False, default_value = None): - ArgValidatorStr.__init__(self, name, required, default_value, None) + ArgValidator.__init__(self, name, required, default_value) self.family = family def _validate(self, value, name): - v = ArgValidatorStr._validate(self, value, name) + if not isinstance(value, Util.STRING_TYPE): + raise ValidationError(name, 'must be a string but is "%s"' % (value)) + v = str(value) + if not v: + raise ValidationError(name, 'cannot be empty') try: return Util.parse_address(v, self.family) except: From 5282966bca72b35462f6599964e8f6cc2ffac8be Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Nov 2017 16:46:01 +0100 Subject: [PATCH 3/9] library: drop redundant "is_v4" property and use address family only Redundancy is bad. --- library/network_connections.py | 19 +++++++++---------- library/test_network_connections.py | 4 ---- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index b8f1421..7c9a5d1 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -275,7 +275,6 @@ class Util: raise MyError('expect two addr-parts: ADDR/PLEN') a, family = Util.parse_ip(addr_parts[0], family) result['address'] = a - result['is_v4'] = (family == socket.AF_INET) result['family'] = family prefix = int(addr_parts[1]) if not (prefix >=0 and prefix <= (32 if family == socket.AF_INET else 128)): @@ -590,7 +589,7 @@ class ArgValidatorIP(ArgValidatorStr): raise ValidationError(name, 'value "%s" is not a valid IP%s address' % (value, Util.addr_family_to_v(self.family))) if self.plain_address: return addr - return { 'is_v4': family == socket.AF_INET, 'family': family, 'address': addr } + return { 'family': family, 'address': addr } class ArgValidatorMac(ArgValidatorStr): def __init__(self, name, force_len = None, required = False, default_value = None): @@ -662,9 +661,9 @@ class ArgValidator_DictIP(ArgValidatorDict): def _validate_post(self, value, name, result): if result['dhcp4'] is None: - result['dhcp4'] = result['dhcp4_send_hostname'] is not None or not any([a for a in result['address'] if a['is_v4']]) + result['dhcp4'] = result['dhcp4_send_hostname'] is not None or not any([a for a in result['address'] if a['family'] == socket.AF_INET]) if result['auto6'] is None: - result['auto6'] = not any([a for a in result['address'] if not a['is_v4']]) + result['auto6'] = not any([a for a in result['address'] if a['family'] == socket.AF_INET6]) if result['dhcp4_send_hostname'] is not None: if not result['dhcp4']: raise ValidationError(name, '"dhcp4_send_hostname" is only valid if "dhcp4" is enabled') @@ -1047,8 +1046,8 @@ class IfcfgUtil: else: raise MyError('invalid slave_type "%s"' % (connection['slave_type'])) else: - addrs4 = list([a for a in ip['address'] if a['is_v4']]) - addrs6 = list([a for a in ip['address'] if not a['is_v4']]) + addrs4 = list([a for a in ip['address'] if a['family'] == socket.AF_INET]) + addrs6 = list([a for a in ip['address'] if a['family'] == socket.AF_INET6]) if ip['dhcp4']: ifcfg['BOOTPROTO'] = 'dhcp' @@ -1382,8 +1381,8 @@ class NMUtil: s_ip4.set_property(NM.SETTING_IP_CONFIG_METHOD, 'auto') s_ip6.set_property(NM.SETTING_IP_CONFIG_METHOD, 'auto') - addrs4 = list([a for a in ip['address'] if a['is_v4']]) - addrs6 = list([a for a in ip['address'] if not a['is_v4']]) + addrs4 = list([a for a in ip['address'] if a['family'] == socket.AF_INET]) + addrs6 = list([a for a in ip['address'] if a['family'] == socket.AF_INET6]) if ip['dhcp4']: s_ip4.set_property(NM.SETTING_IP_CONFIG_METHOD, 'auto') @@ -1399,7 +1398,7 @@ class NMUtil: if ip['route_metric4'] is not None and ip['route_metric4'] >= 0: s_ip4.set_property(NM.SETTING_IP_CONFIG_ROUTE_METRIC, ip['route_metric4']) for d in ip['dns']: - if d['is_v4']: + if d['family'] == socket.AF_INET: s_ip4.add_dns(d['address']) for s in ip['dns_search']: s_ip4.add_dns_search(s) @@ -1417,7 +1416,7 @@ class NMUtil: if ip['route_metric6'] is not None and ip['route_metric6'] >= 0: s_ip6.set_property(NM.SETTING_IP_CONFIG_ROUTE_METRIC, ip['route_metric6']) for d in ip['dns']: - if not d['is_v4']: + if d['family'] == socket.AF_INET6: s_ip6.add_dns(d['address']) try: diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 1206af6..3eeb022 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -256,7 +256,6 @@ class TestValidator(unittest.TestCase): 'dns': [], 'address': [ { - 'is_v4': True, 'prefix': 24, 'family': 2, 'address': '192.168.174.5' @@ -305,13 +304,11 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'address': [ { - 'is_v4': True, 'prefix': 24, 'family': 2, 'address': '192.168.176.5' }, { - 'is_v4': True, 'prefix': 24, 'family': 2, 'address': '192.168.177.5' @@ -356,7 +353,6 @@ class TestValidator(unittest.TestCase): 'dns': [], 'address': [ { - 'is_v4': True, 'prefix': 24, 'family': 2, 'address': '192.168.174.5' From 07c37e5e82c2c113c07b55ce1630a44f5c1c5509 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 14 Nov 2017 18:59:26 +0100 Subject: [PATCH 4/9] library: accept parsing IP address as dictionary in additon to plain string Addresses (currently) have only few properties, so this might not make too much sense in the first moment. However, we will add routes, which have lots of properties. To support routes, we will treat them as dictionaries, not string entries. Hence, for consistency, allow that syntax for addresses as well. --- library/network_connections.py | 78 ++++++++++++++++++++++------- library/test_network_connections.py | 26 +++++++--- 2 files changed, 80 insertions(+), 24 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 7c9a5d1..e89360f 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -243,6 +243,7 @@ class Util: if addr is None: return (None, None) if family is not None: + Util.addr_family_check(family) a = socket.inet_pton(family, addr) else: a = None @@ -255,6 +256,11 @@ class Util: family = socket.AF_INET6 return (socket.inet_ntop(family, a), family) + @staticmethod + def addr_family_check(family): + if family != socket.AF_INET and family != socket.AF_INET6: + raise MyError('invalid address family %s' % (family)) + @staticmethod def addr_family_to_v(family): if family is None: @@ -265,26 +271,43 @@ class Util: return 'v6' raise MyError('invalid address family "%s"' % (family)) + @staticmethod + def addr_family_default_prefix(family): + Util.addr_family_check(family) + if family == socket.AF_INET: + return 24 + else: + return 64 + + @staticmethod + def addr_family_valid_prefix(family, prefix): + Util.addr_family_check(family) + if family == socket.AF_INET: + m = 32 + else: + m = 128 + return prefix >= 0 and prefix <= m + @staticmethod def parse_address(address, family = None): - result = {} try: parts = address.split() addr_parts = parts[0].split('/') if len(addr_parts) != 2: raise MyError('expect two addr-parts: ADDR/PLEN') a, family = Util.parse_ip(addr_parts[0], family) - result['address'] = a - result['family'] = family prefix = int(addr_parts[1]) - if not (prefix >=0 and prefix <= (32 if family == socket.AF_INET else 128)): + if not Util.addr_family_valid_prefix(family, prefix): raise MyError('invalid prefix %s' % (prefix)) - result['prefix'] = prefix if len(parts) > 1: raise MyError('too many parts') + return { + 'address': a, + 'family': family, + 'prefix': prefix, + } except Exception as e: raise MyError('invalid address "%s"' % (address)) - return result ############################################################################### @@ -605,20 +628,41 @@ class ArgValidatorMac(ArgValidatorStr): raise ValidationError(name, 'value "%s" is not a valid MAC address' % (value)) return Util.mac_ntoa(addr) -class ArgValidatorIPAddr(ArgValidator): +class ArgValidatorIPAddr(ArgValidatorDict): def __init__(self, name, family = None, required = False, default_value = None): - ArgValidator.__init__(self, name, required, default_value) + ArgValidatorDict.__init__(self, + name, + required, + nested = [ + ArgValidatorIP ('address', family = family, required = True, plain_address = False), + ArgValidatorNum('prefix', default_value = None, val_min = 0), + ], + ) self.family = family def _validate(self, value, name): - if not isinstance(value, Util.STRING_TYPE): - raise ValidationError(name, 'must be a string but is "%s"' % (value)) - v = str(value) - if not v: - raise ValidationError(name, 'cannot be empty') - try: - return Util.parse_address(v, self.family) - except: - raise ValidationError(name, 'value "%s" is not a valid IP%s address with prefix length' % (value, Util.addr_family_to_v(self.family))) + if isinstance(value, Util.STRING_TYPE): + v = str(value) + if not v: + raise ValidationError(name, 'cannot be empty') + try: + return Util.parse_address(v, self.family) + except: + raise ValidationError(name, 'value "%s" is not a valid IP%s address with prefix length' % (value, Util.addr_family_to_v(self.family))) + v = ArgValidatorDict._validate(self, value, name) + return { + 'address': v['address']['address'], + 'family': v['address']['family'], + 'prefix': v['prefix'], + } + def _validate_post(self, value, name, result): + family = result['family'] + prefix = result['prefix'] + if prefix is None: + prefix = Util.addr_family_default_prefix(family) + result['prefix'] = prefix + elif not Util.addr_family_valid_prefix(family, prefix): + raise ValidationError(name, 'invalid prefix %s in "%s"' % (prefix, value)) + return result class ArgValidator_DictIP(ArgValidatorDict): def __init__(self): diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 3eeb022..28f9c2d 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -3,6 +3,7 @@ import sys import os import unittest +import socket sys.path.insert(1, os.path.dirname(os.path.abspath(__file__))) @@ -257,7 +258,7 @@ class TestValidator(unittest.TestCase): 'address': [ { 'prefix': 24, - 'family': 2, + 'family': socket.AF_INET, 'address': '192.168.174.5' } ] @@ -305,12 +306,12 @@ class TestValidator(unittest.TestCase): 'address': [ { 'prefix': 24, - 'family': 2, + 'family': socket.AF_INET, 'address': '192.168.176.5' }, { 'prefix': 24, - 'family': 2, + 'family': socket.AF_INET, 'address': '192.168.177.5' } ], @@ -349,14 +350,19 @@ class TestValidator(unittest.TestCase): 'dhcp4_send_hostname': None, 'gateway6': None, 'gateway4': None, - 'auto6': True, + 'auto6': False, 'dns': [], 'address': [ { 'prefix': 24, - 'family': 2, + 'family': socket.AF_INET, 'address': '192.168.174.5' - } + }, + { + 'prefix': 65, + 'family': socket.AF_INET6, + 'address': 'a:b:c::6', + }, ] }, 'mac': None, @@ -394,7 +400,13 @@ class TestValidator(unittest.TestCase): 'parent': 'prod1', 'vlan_id': '100', 'ip': { - 'address': [ '192.168.174.5/24' ], + 'address': [ + '192.168.174.5/24', + { + 'address': 'a:b:c::6', + 'prefix': 65, + }, + ], } } ]), From 55e2e43401c18dc2f6f99d3672eccefec9913898 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 16 Nov 2017 13:20:22 +0100 Subject: [PATCH 5/9] library/tests: refactor validating connections Move the asserts to a separate function, so that we can add more checks there. --- library/test_network_connections.py | 96 +++++++++++++++-------------- 1 file changed, 49 insertions(+), 47 deletions(-) diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 28f9c2d..7600dde 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -33,6 +33,13 @@ class TestValidator(unittest.TestCase): v.validate, value) + def do_connections_check_invalid(self, input_connections): + self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, input_connections) + + def do_connections_validate(self, expected_connections, input_connections): + connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + self.assertEqual(expected_connections, connections) + def test_validate_str(self): v = n.ArgValidatorStr('state') @@ -127,12 +134,12 @@ class TestValidator(unittest.TestCase): self.maxDiff = None - self.assertEqual( + self.do_connections_validate( + [], [], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([]), ) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -171,14 +178,14 @@ class TestValidator(unittest.TestCase): 'ignore_errors': None, } ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'type': 'ethernet', }, { 'name': '5' } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -212,18 +219,17 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'state': 'up', 'type': 'ethernet', }, - ]), + ], ) - self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, - [ { 'name': 'a', 'autoconnect': True }]) + self.do_connections_check_invalid([ { 'name': 'a', 'autoconnect': True }]) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -231,15 +237,15 @@ class TestValidator(unittest.TestCase): 'ignore_errors': None, } ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'state': 'absent', } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -279,7 +285,7 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': 'prod1', 'state': 'up', @@ -291,10 +297,10 @@ class TestValidator(unittest.TestCase): 'address': '192.168.174.5/24', } } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -381,7 +387,7 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, } ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': 'prod1', 'state': 'up', @@ -409,10 +415,10 @@ class TestValidator(unittest.TestCase): ], } } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -477,7 +483,7 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, } ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': 'prod2', 'state': 'up', @@ -495,10 +501,10 @@ class TestValidator(unittest.TestCase): 'interface_name': 'eth1', 'master': 'prod2', } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -536,16 +542,16 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': 'bond1', 'state': 'up', 'type': 'bond', }, - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -583,7 +589,7 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': 'bond1', 'state': 'up', @@ -592,15 +598,13 @@ class TestValidator(unittest.TestCase): 'mode': 'active-backup', }, }, - ]), + ], ) - self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, - [ { } ]) - self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, - [ { 'name': 'b', 'xxx': 5 } ]) + self.do_connections_check_invalid([ { } ]) + self.do_connections_check_invalid([ { 'name': 'b', 'xxx': 5 } ]) - self.assertEqual( + self.do_connections_validate( [ { 'autoconnect': True, @@ -632,16 +636,16 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'type': 'ethernet', 'mac': 'AA:bb:cC:DD:ee:FF', } - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -675,15 +679,15 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'state': 'up', 'type': 'ethernet', }, - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -717,17 +721,17 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'state': 'up', 'type': 'ethernet', 'ip': { }, }, - ]), + ], ) - self.assertEqual( + self.do_connections_validate( [ { 'name': '5', @@ -761,7 +765,7 @@ class TestValidator(unittest.TestCase): 'infiniband_transport_mode': None, }, ], - n.AnsibleUtil.ARGS_CONNECTIONS.validate([ + [ { 'name': '5', 'state': 'up', 'type': 'ethernet', @@ -769,12 +773,10 @@ class TestValidator(unittest.TestCase): 'dns_search': [ 'aa', 'bb' ], }, }, - ]), + ], ) - - self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, - [ { 'name': 'b', 'type': 'ethernet', 'mac': 'aa:b' } ]) + self.do_connections_check_invalid([ { 'name': 'b', 'type': 'ethernet', 'mac': 'aa:b' } ]) @my_test_skipIf(nmutil is None, 'no support for NM (libnm via pygobject)') class TestNM(unittest.TestCase): From 6d0b0c1468c8c01c642e4182d2644901bc9e1bcb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 16 Nov 2017 16:48:40 +0100 Subject: [PATCH 6/9] library: add provider specifiy pre-validation step of input Depending on the provider, we have additional restrictions on the input arguments. Validate them early. Without this, we will only fail later when we want to get the 'interface_name' of a parent or a master profile. Fail early. The major reason for this, is to expose this validation so that it can be used by unit tests, to check whether proceeding will lead to a known failure. --- library/network_connections.py | 41 +++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/library/network_connections.py b/library/network_connections.py index e89360f..f698440 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -48,6 +48,10 @@ class ValidationError(MyError): self.error_message = message self.name = name + @staticmethod + def from_connection(idx, message): + return ValidationError('connection[' + str(idx) + ']', message) + class Util: PY3 = (sys.version_info[0] == 3) @@ -442,7 +446,6 @@ class ArgUtil: c = ArgUtil.connection_find_by_name(name, connections, n_connections) if not c: raise MyError('invalid master/parent "%s"' % (name)) - assert c.get('nm.uuid', None) return c['nm.uuid'] @staticmethod @@ -926,6 +929,31 @@ class ArgValidator_ListConnections(ArgValidatorList): raise ValidationError(name + '[' + str(idx) + '].parent', 'references non-existing "parent" connection "%s"' % (connection['parent'])) return result + VALIDATE_ONE_MODE_NM = 'nm' + VALIDATE_ONE_MODE_INITSCRIPTS = 'initscripts' + + def validate_connection_one(self, mode, connections, idx): + connection = connections[idx] + if 'type' not in connection: + return + + if ( (connection['parent']) + and ( ( (mode == self.VALIDATE_ONE_MODE_INITSCRIPTS) + and (connection['type'] == 'vlan')) + or ( (connection['type'] == 'infiniband') + and (connection['infiniband_p_key'] not in [ None, -1 ])))): + try: + ArgUtil.connection_find_master(connection['parent'], connections, idx) + except MyError as e: + raise ValidationError.from_connection(idx, 'profile references a parent "%s" which has \'interface_name\' missing' % (connection['parent'])) + + if ( (connection['master']) + and (mode == self.VALIDATE_ONE_MODE_INITSCRIPTS)): + try: + ArgUtil.connection_find_master(connection['master'], connections, idx) + except MyError as e: + raise ValidationError.from_connection(idx, 'profile references a master "%s" which has \'interface_name\' missing' % (connection['master'])) + ############################################################################### class IfcfgUtil: @@ -1972,6 +2000,13 @@ class Cmd: AnsibleUtil.fail_json('unsupported provider %s' % (provider)) def run(self): + for idx, connection in enumerate(AnsibleUtil.connections): + try: + AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(self.validate_one_type, + AnsibleUtil.connections, + idx) + except ValidationError as e: + AnsibleUtil.log_fatal(idx, str(e)) self.run_prepare() while AnsibleUtil.check_mode_next() != CheckMode.DONE: for idx, connection in enumerate(AnsibleUtil.connections): @@ -2033,6 +2068,7 @@ class Cmd_nm(Cmd): def __init__(self): self._nmutil = None + self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_NM @property def nmutil(self): @@ -2222,6 +2258,9 @@ class Cmd_nm(Cmd): class Cmd_initscripts(Cmd): + def __init__(self): + self.validate_one_type = ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS + def check_name(self, idx, name = None): if name is None: name = AnsibleUtil.connections[idx]['name'] From 3df0cb77c1a92d15b634e7130d3a19ba0b9dc240 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 16 Nov 2017 13:20:22 +0100 Subject: [PATCH 7/9] library/tests: test creating NM connections --- library/network_connections.py | 2 +- library/test_network_connections.py | 49 ++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 5 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index f698440..46db220 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -1045,7 +1045,7 @@ class IfcfgUtil: return s @classmethod - def ifcfg_create(cls, connections, idx, warn_fcn): + def ifcfg_create(cls, connections, idx, warn_fcn = lambda msg: None): connection = connections[idx] ip = connection['ip'] diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 7600dde..f7293ea 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -21,11 +21,24 @@ except AttributeError: try: nmutil = n.NMUtil() + assert(nmutil) except: # NMUtil is not supported, for example on RHEL 6 or without # pygobject. nmutil = None +if nmutil: + NM = n.Util.NM() + GObject = n.Util.GObject() + +def pprint(msg, obj): + print('PRINT: %s\n' % (msg)) + import pprint + p = pprint.PrettyPrinter(indent = 4) + p.pprint(obj) + if nmutil is not None and isinstance(obj, NM.Connection): + obj.dump() + class TestValidator(unittest.TestCase): def assertValidationError(self, v, value): @@ -36,9 +49,41 @@ class TestValidator(unittest.TestCase): def do_connections_check_invalid(self, input_connections): self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, input_connections) + def do_connections_validate_nm(self, input_connections): + if not nmutil: + return + connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + for connection in connections: + if 'type' in connection: + connection['nm.exists'] = False + connection['nm.uuid'] = n.Util.create_uuid() + mode = n.ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS + for idx, connection in enumerate(connections): + try: + n.AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) + except n.ValidationError as e: + continue + if 'type' in connection: + con_new = nmutil.connection_create(connections, idx) + self.assertTrue(con_new) + self.assertTrue(con_new.verify()) + + def do_connections_validate_ifcfg(self, input_connections): + mode = n.ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS + connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) + for idx, connection in enumerate(connections): + try: + n.AnsibleUtil.ARGS_CONNECTIONS.validate_connection_one(mode, connections, idx) + except n.ValidationError as e: + continue + if 'type' in connection: + c = n.IfcfgUtil.ifcfg_create(connections, idx) + def do_connections_validate(self, expected_connections, input_connections): connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) self.assertEqual(expected_connections, connections) + self.do_connections_validate_nm(input_connections) + self.do_connections_validate_ifcfg(input_connections) def test_validate_str(self): @@ -782,10 +827,6 @@ class TestValidator(unittest.TestCase): class TestNM(unittest.TestCase): def test_connection_ensure_setting(self): - - NM = n.Util.NM() - GObject = n.Util.GObject() - con = NM.SimpleConnection.new() self.assertIsNotNone(con) self.assertTrue(GObject.type_is_a(con, NM.Connection)) From 936b5c93cc33573b018736e131d18b887c7a90ad Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 15 Nov 2017 17:32:44 +0100 Subject: [PATCH 8/9] library: add support for static routes --- library/network_connections.py | 180 ++++++++++++++--- library/test_network_connections.py | 302 ++++++++++++++++++++++++++-- 2 files changed, 438 insertions(+), 44 deletions(-) diff --git a/library/network_connections.py b/library/network_connections.py index 46db220..a86c5fa 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -667,6 +667,41 @@ class ArgValidatorIPAddr(ArgValidatorDict): raise ValidationError(name, 'invalid prefix %s in "%s"' % (prefix, value)) return result +class ArgValidatorIPRoute(ArgValidatorDict): + def __init__(self, name, family = None, required = False, default_value = None): + ArgValidatorDict.__init__(self, + name, + required, + nested = [ + ArgValidatorIP ('network', family = family, required = True, plain_address = False), + ArgValidatorNum('prefix', default_value = None, val_min = 0), + ArgValidatorIP ('gateway', family = family, default_value = None, plain_address = False), + ArgValidatorNum('metric', default_value = -1, val_min = -1, val_max = 0xFFFFFFFF), + ], + ) + self.family = family + def _validate_post(self, value, name, result): + network = result['network'] + + family = network['family'] + result['network'] = network['address'] + result['family'] = family + + gateway = result['gateway'] + if gateway is not None: + if family != gateway['family']: + raise ValidationError(name, 'conflicting address family between network and gateway \"%s\"' % (gateway['address'])) + result['gateway'] = gateway['address'] + + prefix = result['prefix'] + if prefix is None: + prefix = Util.addr_family_default_prefix(family) + result['prefix'] = prefix + elif not Util.addr_family_valid_prefix(family, prefix): + raise ValidationError(name, 'invalid prefix %s in "%s"' % (prefix, value)) + + return result + class ArgValidator_DictIP(ArgValidatorDict): def __init__(self): ArgValidatorDict.__init__(self, @@ -683,6 +718,11 @@ class ArgValidator_DictIP(ArgValidatorDict): nested = ArgValidatorIPAddr('address[?]'), default_value = list, ), + ArgValidatorList('route', + nested = ArgValidatorIPRoute('route[?]'), + default_value = list, + ), + ArgValidatorBool('route_append_only'), ArgValidatorList('dns', nested = ArgValidatorIP('dns[?]', plain_address=False), default_value = list, @@ -701,6 +741,8 @@ class ArgValidator_DictIP(ArgValidatorDict): 'gateway6': None, 'route_metric6': None, 'address': [], + 'route': [], + 'route_append_only': False, 'dns': [], 'dns_search': [], }, @@ -1045,14 +1087,40 @@ class IfcfgUtil: return s @classmethod - def ifcfg_create(cls, connections, idx, warn_fcn = lambda msg: None): + def _ifcfg_route_merge(cls, route, append_only, current): + if not append_only or current is None: + if not route: + return None + return '\n'.join(route) + '\n' + + if route: + # the 'route' file is processed line by line by initscripts' ifup-route. Hence, + # the order of the route matters. _ifcfg_route_merge() is not sophisticated + # enough to understand pre-existing lines. It will only append lines that + # don't exist yet, which hopefully is correct. + # It's better to always rewrite the entire file with route_append_only=False. + changed = False + c_lines = list(current.split('\n')) + for r in route: + if r not in c_lines: + changed = True + c_lines.append(r) + if changed: + return '\n'.join(c_lines) + '\n' + + return current + + @classmethod + def ifcfg_create(cls, connections, idx, warn_fcn = lambda msg: None, content_current = None): connection = connections[idx] ip = connection['ip'] - ifcfg_all = {} - for file_type in cls.FILE_TYPES: - ifcfg_all[file_type] = {} - ifcfg = ifcfg_all['ifcfg'] + ifcfg = {} + keys_file = None + route4_file = None + route6_file = None + rule4_file = None + rule6_file = None if ip['dhcp4_send_hostname'] is not None: warn_fcn('ip.dhcp4_send_hostname is not supported by initscripts provider') @@ -1117,6 +1185,10 @@ class IfcfgUtil: ifcfg['DEVICETYPE'] = 'TeamPort' else: raise MyError('invalid slave_type "%s"' % (connection['slave_type'])) + + if ip['route_append_only'] and content_current: + route4_file = content_current['route'] + route6_file = content_current['route6'] else: addrs4 = list([a for a in ip['address'] if a['family'] == socket.AF_INET]) addrs6 = list([a for a in ip['address'] if a['family'] == socket.AF_INET6]) @@ -1154,16 +1226,43 @@ class IfcfgUtil: if ip['gateway6'] is not None: ifcfg['IPV6_DEFAULTGW'] = ip['gateway6'] - for file_type in cls.FILE_TYPES: - h = ifcfg_all[file_type] - for key in h.keys(): - if h[key] is None: - del h[key] - continue - if type(h[key]) == type(True): - h[key] = 'yes' if h[key] else 'no' + route4 = [] + route6 = [] + for r in ip['route']: + line = r['network'] + '/' + str(r['prefix']) + if r['gateway']: + line += ' via ' + r['gateway'] + if r['metric'] != -1: + line += ' metric ' + str(r['metric']) - return ifcfg_all + if r['family'] == socket.AF_INET: + route4.append(line) + else: + route6.append(line) + + route4_file = cls._ifcfg_route_merge(route4, + ip['route_append_only'] and content_current, + content_current['route'] if content_current else None) + route6_file = cls._ifcfg_route_merge(route6, + ip['route_append_only'] and content_current, + content_current['route6'] if content_current else None) + + for key in list(ifcfg.keys()): + v = ifcfg[key] + if v is None: + del ifcfg[key] + continue + if type(v) == type(True): + ifcfg[key] = 'yes' if v else 'no' + + return { + 'ifcfg': ifcfg, + 'keys': keys_file, + 'route': route4_file, + 'route6': route6_file, + 'rule': rule4_file, + 'rule6': rule6_file, + } @classmethod def ifcfg_parse_line(cls, line): @@ -1210,18 +1309,18 @@ class IfcfgUtil: content = {} for file_type in cls._file_types(file_type): h = ifcfg_all[file_type] - if not h: - if file_type != 'ifcfg': - content[file_type] = None - continue - s = cls.ANSIBLE_MANAGED + '\n' - for key in sorted(h.keys()): - value = h[key] - if not cls.KeyValid(key): - raise MyError('invalid ifcfg key %s' % (key)) - if value is not None: - s += key + '=' + cls.ValueEscape(value) + '\n' - content[file_type] = s + if file_type == 'ifcfg': + s = cls.ANSIBLE_MANAGED + '\n' + for key in sorted(h.keys()): + value = h[key] + if not cls.KeyValid(key): + raise MyError('invalid ifcfg key %s' % (key)) + if value is not None: + s += key + '=' + cls.ValueEscape(value) + '\n' + content[file_type] = s + else: + content[file_type] = h + return content @classmethod @@ -1298,6 +1397,11 @@ class NMUtil: nmclient = Util.NM().Client.new(None) self.nmclient = nmclient + def setting_ip_config_get_routes(self, s_ip): + if s_ip is not None: + for i in range(0, s_ip.get_num_routes()): + yield s_ip.get_route(i) + def connection_ensure_setting(self, connection, setting_type): setting = connection.get_setting(setting_type) if not setting: @@ -1388,7 +1492,7 @@ class NMUtil: return True return False - def connection_create(self, connections, idx): + def connection_create(self, connections, idx, connection_current = None): NM = Util.NM() connection = connections[idx] @@ -1491,6 +1595,18 @@ class NMUtil: if d['family'] == socket.AF_INET6: s_ip6.add_dns(d['address']) + if ip['route_append_only'] and connection_current: + for r in self.setting_ip_config_get_routes(connection_current.get_setting(NM.SettingIP4Config)): + s_ip4.add_route(r) + for r in self.setting_ip_config_get_routes(connection_current.get_setting(NM.SettingIP6Config)): + s_ip6.add_route(r) + for r in ip['route']: + rr = NM.IPRoute.new(r['family'], r['network'], r['prefix'], r['gateway'], r['metric']) + if r['family'] == socket.AF_INET: + s_ip4.add_route(rr) + else: + s_ip6.add_route(rr) + try: con.normalize() except Exception as e: @@ -2135,7 +2251,7 @@ class Cmd_nm(Cmd): def run_state_present(self, idx): connection = AnsibleUtil.connections[idx] con_cur = Util.first(self.nmutil.connection_list(name = connection['name'], uuid = connection['nm.uuid'])) - con_new = self.nmutil.connection_create(AnsibleUtil.connections, idx) + con_new = self.nmutil.connection_create(AnsibleUtil.connections, idx, con_cur) changed = False if con_cur is None: AnsibleUtil.log_info(idx, 'add connection %s, %s' % (connection['name'], connection['nm.uuid'])) @@ -2314,10 +2430,12 @@ class Cmd_initscripts(Cmd): connection = AnsibleUtil.connections[idx] name = connection['name'] - ifcfg_all = IfcfgUtil.ifcfg_create(AnsibleUtil.connections, idx, - lambda msg: AnsibleUtil.log_warn(idx, msg)) - old_content = IfcfgUtil.content_from_file(name) + + ifcfg_all = IfcfgUtil.ifcfg_create(AnsibleUtil.connections, idx, + lambda msg: AnsibleUtil.log_warn(idx, msg), + old_content) + new_content = IfcfgUtil.content_from_dict(ifcfg_all) if old_content == new_content: diff --git a/library/test_network_connections.py b/library/test_network_connections.py index f7293ea..7bb49e0 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -4,6 +4,7 @@ import sys import os import unittest import socket +import itertools sys.path.insert(1, os.path.dirname(os.path.abspath(__file__))) @@ -46,10 +47,24 @@ class TestValidator(unittest.TestCase): v.validate, value) + def assert_nm_connection_routes_expected(self, connection, route_list_expected): + parser = n.ArgValidatorIPRoute('route[?]') + route_list_exp = [parser.validate(r) for r in route_list_expected] + route_list_new = itertools.chain(nmutil.setting_ip_config_get_routes(connection.get_setting(NM.SettingIP4Config)), + nmutil.setting_ip_config_get_routes(connection.get_setting(NM.SettingIP6Config))) + route_list_new = [{ + 'family': r.get_family(), + 'network': r.get_dest(), + 'prefix': int(r.get_prefix()), + 'gateway': r.get_next_hop(), + 'metric': int(r.get_metric()), + } for r in route_list_new] + self.assertEqual(route_list_exp, route_list_new) + def do_connections_check_invalid(self, input_connections): self.assertValidationError(n.AnsibleUtil.ARGS_CONNECTIONS, input_connections) - def do_connections_validate_nm(self, input_connections): + def do_connections_validate_nm(self, input_connections, **kwargs): if not nmutil: return connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) @@ -67,8 +82,27 @@ class TestValidator(unittest.TestCase): con_new = nmutil.connection_create(connections, idx) self.assertTrue(con_new) self.assertTrue(con_new.verify()) + if 'nm_route_list_current' in kwargs: + parser = n.ArgValidatorIPRoute('route[?]') + s4 = con_new.get_setting(NM.SettingIP4Config) + s6 = con_new.get_setting(NM.SettingIP6Config) + s4.clear_routes() + s6.clear_routes() + for r in kwargs['nm_route_list_current'][idx]: + r = parser.validate(r) + r = NM.IPRoute.new(r['family'], r['network'], r['prefix'], r['gateway'], r['metric']) + if r.get_family() == socket.AF_INET: + s4.add_route(r) + else: + s6.add_route(r) + con_new = nmutil.connection_create(connections, idx, + connection_current = con_new) + self.assertTrue(con_new) + self.assertTrue(con_new.verify()) + if 'nm_route_list_expected' in kwargs: + self.assert_nm_connection_routes_expected(con_new, kwargs['nm_route_list_expected'][idx]) - def do_connections_validate_ifcfg(self, input_connections): + def do_connections_validate_ifcfg(self, input_connections, **kwargs): mode = n.ArgValidator_ListConnections.VALIDATE_ONE_MODE_INITSCRIPTS connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) for idx, connection in enumerate(connections): @@ -77,13 +111,21 @@ class TestValidator(unittest.TestCase): except n.ValidationError as e: continue if 'type' in connection: - c = n.IfcfgUtil.ifcfg_create(connections, idx) + content_current = kwargs.get('initscripts_content_current', None) + if content_current: + content_current = content_current[idx] + c = n.IfcfgUtil.ifcfg_create(connections, idx, content_current = content_current) + #pprint("con[%s] = \"%s\"" % (idx, connections[idx]['name']), c) + exp = kwargs.get('initscripts_dict_expected', None) + if exp is not None: + self.assertEqual(exp[idx], c) - def do_connections_validate(self, expected_connections, input_connections): + + def do_connections_validate(self, expected_connections, input_connections, **kwargs): connections = n.AnsibleUtil.ARGS_CONNECTIONS.validate(input_connections) self.assertEqual(expected_connections, connections) - self.do_connections_validate_nm(input_connections) - self.do_connections_validate_ifcfg(input_connections) + self.do_connections_validate_nm(input_connections, **kwargs) + self.do_connections_validate_ifcfg(input_connections, **kwargs) def test_validate_str(self): @@ -199,6 +241,8 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'dhcp4': True, 'address': [], + 'route_append_only': False, + 'route': [], 'route_metric6': None, 'dhcp4_send_hostname': None, 'dns': [], @@ -245,6 +289,8 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'dhcp4': True, 'address': [], + 'route_append_only': False, + 'route': [], 'dns': [], 'dns_search': [], 'route_metric6': None, @@ -312,7 +358,9 @@ class TestValidator(unittest.TestCase): 'family': socket.AF_INET, 'address': '192.168.174.5' } - ] + ], + 'route_append_only': False, + 'route': [], }, 'state': 'up', 'mtu': 1450, @@ -366,6 +414,8 @@ class TestValidator(unittest.TestCase): 'address': '192.168.177.5' } ], + 'route_append_only': False, + 'route': [], 'route_metric6': None, 'route_metric4': None, 'dns_search': [], @@ -414,7 +464,17 @@ class TestValidator(unittest.TestCase): 'family': socket.AF_INET6, 'address': 'a:b:c::6', }, - ] + ], + 'route_append_only': False, + 'route': [ + { + 'family': socket.AF_INET, + 'network': '192.168.5.0', + 'prefix': 24, + 'gateway': None, + 'metric': -1, + }, + ], }, 'mac': None, 'mtu': None, @@ -458,6 +518,12 @@ class TestValidator(unittest.TestCase): 'prefix': 65, }, ], + 'route_append_only': False, + 'route': [ + { + 'network': '192.168.5.0', + }, + ], } } ], @@ -479,7 +545,9 @@ class TestValidator(unittest.TestCase): 'gateway4': None, 'auto6': False, 'dns': [], - 'address': [] + 'address': [], + 'route_append_only': False, + 'route': [], }, 'mac': None, 'mtu': None, @@ -504,6 +572,8 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'auto6': True, 'address': [], + 'route_append_only': False, + 'route': [], 'route_metric6': None, 'route_metric4': None, 'dns_search': [], @@ -565,7 +635,9 @@ class TestValidator(unittest.TestCase): 'gateway4': None, 'auto6': True, 'dns': [], - 'address': [] + 'address': [], + 'route_append_only': False, + 'route': [], }, 'mac': None, 'mtu': None, @@ -612,7 +684,9 @@ class TestValidator(unittest.TestCase): 'gateway4': None, 'auto6': True, 'dns': [], - 'address': [] + 'address': [], + 'route_append_only': False, + 'route': [], }, 'mac': None, 'mtu': None, @@ -656,6 +730,8 @@ class TestValidator(unittest.TestCase): 'interface_name': None, 'ip': { 'address': [], + 'route_append_only': False, + 'route': [], 'auto6': True, 'dhcp4': True, 'dhcp4_send_hostname': None, @@ -705,6 +781,8 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'dhcp4': True, 'address': [], + 'route_append_only': False, + 'route': [], 'dns': [], 'dns_search': [ ], 'route_metric6': None, @@ -747,6 +825,8 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'dhcp4': True, 'address': [], + 'route_append_only': False, + 'route': [], 'dns': [], 'dns_search': [ ], 'route_metric6': None, @@ -779,7 +859,7 @@ class TestValidator(unittest.TestCase): self.do_connections_validate( [ { - 'name': '5', + 'name': '555', 'state': 'up', 'type': 'ethernet', 'autoconnect': True, @@ -791,6 +871,23 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'dhcp4': True, 'address': [], + 'route_append_only': False, + 'route': [ + { + 'family': socket.AF_INET, + 'network': '192.168.45.0', + 'prefix': 24, + 'gateway': None, + 'metric': 545, + }, + { + 'family': socket.AF_INET, + 'network': '192.168.46.0', + 'prefix': 30, + 'gateway': None, + 'metric': -1, + }, + ], 'dns': [], 'dns_search': [ 'aa', 'bb' ], 'route_metric6': None, @@ -811,14 +908,193 @@ class TestValidator(unittest.TestCase): }, ], [ - { 'name': '5', + { 'name': '555', 'state': 'up', 'type': 'ethernet', 'ip': { 'dns_search': [ 'aa', 'bb' ], + 'route': [ + { + 'network': '192.168.45.0', + 'metric': 545, + }, + { + 'network': '192.168.46.0', + 'prefix': 30, + }, + ], }, }, ], + initscripts_dict_expected = [ + { + 'ifcfg': { + 'BOOTPROTO': 'dhcp', + 'DOMAIN': 'aa bb', + 'IPV6INIT': 'yes', + 'IPV6_AUTOCONF': 'yes', + 'NM_CONTROLLED': 'no', + 'ONBOOT': 'yes', + 'TYPE': 'Ethernet', + }, + 'keys': None, + 'route': '192.168.45.0/24 metric 545\n192.168.46.0/30\n', + 'route6': None, + 'rule': None, + 'rule6': None, + }, + ], + ) + + self.do_connections_validate( + [ + { + 'name': 'e556', + 'state': 'up', + 'type': 'ethernet', + 'autoconnect': True, + 'parent': None, + 'ip': { + 'gateway6': None, + 'gateway4': None, + 'route_metric4': None, + 'auto6': True, + 'dhcp4': True, + 'address': [], + 'route_append_only': True, + 'route': [ + { + 'family': socket.AF_INET, + 'network': '192.168.45.0', + 'prefix': 24, + 'gateway': None, + 'metric': 545, + }, + { + 'family': socket.AF_INET, + 'network': '192.168.46.0', + 'prefix': 30, + 'gateway': None, + 'metric': -1, + }, + { + 'family': socket.AF_INET6, + 'network': 'a:b:c:d::', + 'prefix': 64, + 'gateway': None, + 'metric': -1, + }, + ], + 'dns': [], + 'dns_search': [ 'aa', 'bb' ], + 'route_metric6': None, + 'dhcp4_send_hostname': None, + }, + 'mac': None, + 'mtu': None, + 'master': None, + 'vlan_id': None, + 'ignore_errors': None, + 'interface_name': None, + 'check_iface_exists': True, + 'force_state_change': None, + 'slave_type': None, + 'wait': None, + 'infiniband_p_key': None, + 'infiniband_transport_mode': None, + }, + ], + [ + { 'name': 'e556', + 'state': 'up', + 'type': 'ethernet', + 'ip': { + 'dns_search': [ 'aa', 'bb' ], + 'route_append_only': True, + 'route': [ + { + 'network': '192.168.45.0', + 'metric': 545, + }, + { + 'network': '192.168.46.0', + 'prefix': 30, + }, + { + 'network': 'a:b:c:d::', + }, + ], + }, + }, + ], + nm_route_list_current = [ + [ + { + 'network': '192.168.40.0', + 'prefix': 24, + 'metric': 545, + }, + { + 'network': '192.168.46.0', + 'prefix': 30, + }, + { + 'network': 'a:b:c:f::', + }, + ], + ], + nm_route_list_expected = [ + [ + { + 'network': '192.168.40.0', + 'prefix': 24, + 'metric': 545, + }, + { + 'network': '192.168.46.0', + 'prefix': 30, + }, + { + 'network': '192.168.45.0', + 'prefix': 24, + 'metric': 545, + }, + { + 'network': 'a:b:c:f::', + }, + { + 'network': 'a:b:c:d::', + }, + ], + ], + initscripts_content_current = [ + { + 'ifcfg': '', + 'keys': None, + 'route': '192.168.40.0/24 metric 545\n192.168.46.0/30', + 'route6': 'a:b:c:f::/64', + 'rule': None, + 'rule6': None, + }, + ], + initscripts_dict_expected = [ + { + 'ifcfg': { + 'BOOTPROTO': 'dhcp', + 'DOMAIN': 'aa bb', + 'IPV6INIT': 'yes', + 'IPV6_AUTOCONF': 'yes', + 'NM_CONTROLLED': 'no', + 'ONBOOT': 'yes', + 'TYPE': 'Ethernet', + }, + 'keys': None, + 'route': '192.168.40.0/24 metric 545\n192.168.46.0/30\n192.168.45.0/24 metric 545\n', + 'route6': 'a:b:c:f::/64\na:b:c:d::/64\n', + 'rule': None, + 'rule6': None, + }, + ], ) self.do_connections_check_invalid([ { 'name': 'b', 'type': 'ethernet', 'mac': 'aa:b' } ]) From 9740d8d024bc079d151baa4d9802ee537282642f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 17 Nov 2017 13:23:28 +0100 Subject: [PATCH 9/9] library: add 'rule_append_only' setting It works like 'route_append_only' and can be used to prevent the role from deleting rule files. That makes especaily sense, because the role currently doesn't support routing rules. Also, NetworkManager still doesn't support routing rules either. One day, the role (and maybe NetworkManager) will support rules, and at that point it will start configuring them. That is the reason why the new option already defaults to 'False'. Because, once we add support for rules, we want the role to manage them by default. --- library/network_connections.py | 6 ++++++ library/test_network_connections.py | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/library/network_connections.py b/library/network_connections.py index a86c5fa..648b8f3 100755 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -723,6 +723,7 @@ class ArgValidator_DictIP(ArgValidatorDict): default_value = list, ), ArgValidatorBool('route_append_only'), + ArgValidatorBool('rule_append_only'), ArgValidatorList('dns', nested = ArgValidatorIP('dns[?]', plain_address=False), default_value = list, @@ -743,6 +744,7 @@ class ArgValidator_DictIP(ArgValidatorDict): 'address': [], 'route': [], 'route_append_only': False, + 'rule_append_only': False, 'dns': [], 'dns_search': [], }, @@ -1247,6 +1249,10 @@ class IfcfgUtil: ip['route_append_only'] and content_current, content_current['route6'] if content_current else None) + if ip['rule_append_only'] and content_current: + rule4_file = content_current['rule'] + rule6_file = content_current['rule6'] + for key in list(ifcfg.keys()): v = ifcfg[key] if v is None: diff --git a/library/test_network_connections.py b/library/test_network_connections.py index 7bb49e0..740e747 100755 --- a/library/test_network_connections.py +++ b/library/test_network_connections.py @@ -242,6 +242,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'route_metric6': None, 'dhcp4_send_hostname': None, @@ -290,6 +291,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'dns': [], 'dns_search': [], @@ -360,6 +362,7 @@ class TestValidator(unittest.TestCase): } ], 'route_append_only': False, + 'rule_append_only': False, 'route': [], }, 'state': 'up', @@ -415,6 +418,7 @@ class TestValidator(unittest.TestCase): } ], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'route_metric6': None, 'route_metric4': None, @@ -466,6 +470,7 @@ class TestValidator(unittest.TestCase): }, ], 'route_append_only': False, + 'rule_append_only': False, 'route': [ { 'family': socket.AF_INET, @@ -519,6 +524,7 @@ class TestValidator(unittest.TestCase): }, ], 'route_append_only': False, + 'rule_append_only': False, 'route': [ { 'network': '192.168.5.0', @@ -547,6 +553,7 @@ class TestValidator(unittest.TestCase): 'dns': [], 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], }, 'mac': None, @@ -573,6 +580,7 @@ class TestValidator(unittest.TestCase): 'auto6': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'route_metric6': None, 'route_metric4': None, @@ -637,6 +645,7 @@ class TestValidator(unittest.TestCase): 'dns': [], 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], }, 'mac': None, @@ -686,6 +695,7 @@ class TestValidator(unittest.TestCase): 'dns': [], 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], }, 'mac': None, @@ -731,6 +741,7 @@ class TestValidator(unittest.TestCase): 'ip': { 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'auto6': True, 'dhcp4': True, @@ -782,6 +793,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'dns': [], 'dns_search': [ ], @@ -826,6 +838,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [], 'dns': [], 'dns_search': [ ], @@ -872,6 +885,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': False, + 'rule_append_only': False, 'route': [ { 'family': socket.AF_INET, @@ -962,6 +976,7 @@ class TestValidator(unittest.TestCase): 'dhcp4': True, 'address': [], 'route_append_only': True, + 'rule_append_only': False, 'route': [ { 'family': socket.AF_INET, @@ -1011,6 +1026,7 @@ class TestValidator(unittest.TestCase): 'ip': { 'dns_search': [ 'aa', 'bb' ], 'route_append_only': True, + 'rule_append_only': False, 'route': [ { 'network': '192.168.45.0',