From 182c365d877ab224b790f1c36e8735f30a89c111 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Tue, 5 Apr 2022 07:12:38 +0200 Subject: [PATCH] nmcli: suggest new routes4 and routes6 format (#4328) (#4447) * suggest new routes4 and routes6 format * make new options instead of modifying exiting one * fix docs and some small errors * fixing docs (cherry picked from commit feb0fffd584c871d23d0e531e29e6185824a0211) Co-authored-by: Alex Groshev <38885591+haddystuff@users.noreply.github.com> --- ..._reports_changed_for_routes4_parameter.yml | 4 + plugins/modules/net_tools/nmcli.py | 162 ++++++++++++++++-- .../plugins/modules/net_tools/test_nmcli.py | 63 +++++++ 3 files changed, 217 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/4131-nmcli_fix_reports_changed_for_routes4_parameter.yml diff --git a/changelogs/fragments/4131-nmcli_fix_reports_changed_for_routes4_parameter.yml b/changelogs/fragments/4131-nmcli_fix_reports_changed_for_routes4_parameter.yml new file mode 100644 index 0000000000..16a2f3056b --- /dev/null +++ b/changelogs/fragments/4131-nmcli_fix_reports_changed_for_routes4_parameter.yml @@ -0,0 +1,4 @@ +--- +bugfixes: + - nmcli - fix returning "changed" when routes parameters set, also suggest new routes4 and routes6 format + (https://github.com/ansible-collections/community.general/issues/4131). diff --git a/plugins/modules/net_tools/nmcli.py b/plugins/modules/net_tools/nmcli.py index 0a2f113cda..6a6b3c548f 100644 --- a/plugins/modules/net_tools/nmcli.py +++ b/plugins/modules/net_tools/nmcli.py @@ -90,11 +90,53 @@ options: version_added: 3.2.0 routes4: description: - - The list of ipv4 routes. - - Use the format '192.0.3.0/24 192.0.2.1' + - The list of IPv4 routes. + - Use the format C(192.0.3.0/24 192.0.2.1). + - To specify more complex routes, use the I(routes4_extended) option. type: list elements: str version_added: 2.0.0 + routes4_extended: + description: + - The list of IPv4 routes. + type: list + elements: dict + suboptions: + ip: + description: + - IP or prefix of route. + - Use the format C(192.0.3.0/24). + type: str + required: true + next_hop: + description: + - Use the format C(192.0.2.1). + type: str + metric: + description: + - Route metric. + type: int + table: + description: + - The table to add this route to. + - The default depends on C(ipv4.route-table). + type: int + cwnd: + description: + - The clamp for congestion window. + type: int + mtu: + description: + - If non-zero, only transmit packets of the specified size or smaller. + type: int + onlink: + description: + - Pretend that the nexthop is directly attached to this link, even if it does not match any interface prefix. + type: bool + tos: + description: + - The Type Of Service. + type: int route_metric4: description: - Set metric level of ipv4 routes configured on interface. @@ -165,9 +207,47 @@ options: description: - The list of IPv6 routes. - Use the format C(fd12:3456:789a:1::/64 2001:dead:beef::1). + - To specify more complex routes, use the I(routes6_extended) option. type: list elements: str version_added: 4.4.0 + routes6_extended: + description: + - The list of IPv6 routes but with parameters. + type: list + elements: dict + suboptions: + ip: + description: + - IP or prefix of route. + - Use the format C(fd12:3456:789a:1::/64). + type: str + required: true + next_hop: + description: + - Use the format C(2001:dead:beef::1). + type: str + metric: + description: + - Route metric. + type: int + table: + description: + - The table to add this route to. + - The default depends on C(ipv6.route-table). + type: int + cwnd: + description: + - The clamp for congestion window. + type: int + mtu: + description: + - If non-zero, only transmit packets of the specified size or smaller. + type: int + onlink: + description: + - Pretend that the nexthop is directly attached to this link, even if it does not match any interface prefix. + type: bool route_metric6: description: - Set metric level of IPv6 routes configured on interface. @@ -1260,6 +1340,7 @@ class Nmcli(object): self.gw4 = module.params['gw4'] self.gw4_ignore_auto = module.params['gw4_ignore_auto'] self.routes4 = module.params['routes4'] + self.routes4_extended = module.params['routes4_extended'] self.route_metric4 = module.params['route_metric4'] self.routing_rules4 = module.params['routing_rules4'] self.never_default4 = module.params['never_default4'] @@ -1272,6 +1353,7 @@ class Nmcli(object): self.gw6 = module.params['gw6'] self.gw6_ignore_auto = module.params['gw6_ignore_auto'] self.routes6 = module.params['routes6'] + self.routes6_extended = module.params['routes6_extended'] self.route_metric6 = module.params['route_metric6'] self.dns6 = module.params['dns6'] self.dns6_search = module.params['dns6_search'] @@ -1371,7 +1453,7 @@ class Nmcli(object): 'ipv4.ignore-auto-dns': self.dns4_ignore_auto, 'ipv4.gateway': self.gw4, 'ipv4.ignore-auto-routes': self.gw4_ignore_auto, - 'ipv4.routes': self.routes4, + 'ipv4.routes': self.enforce_routes_format(self.routes4, self.routes4_extended), 'ipv4.route-metric': self.route_metric4, 'ipv4.routing-rules': self.routing_rules4, 'ipv4.never-default': self.never_default4, @@ -1383,7 +1465,7 @@ class Nmcli(object): 'ipv6.ignore-auto-dns': self.dns6_ignore_auto, 'ipv6.gateway': self.gw6, 'ipv6.ignore-auto-routes': self.gw6_ignore_auto, - 'ipv6.routes': self.routes6, + 'ipv6.routes': self.enforce_routes_format(self.routes6, self.routes6_extended), 'ipv6.route-metric': self.route_metric6, 'ipv6.method': self.ipv6_method, 'ipv6.ip6-privacy': self.ip_privacy6, @@ -1614,6 +1696,29 @@ class Nmcli(object): return None return [address if '/' in address else address + '/128' for address in ip6_addresses] + def enforce_routes_format(self, routes, routes_extended): + if routes is not None: + return routes + elif routes_extended is not None: + return [self.route_to_string(route) for route in routes_extended] + else: + return None + + @staticmethod + def route_to_string(route): + result_str = '' + result_str += route['ip'] + if route.get('next_hop') is not None: + result_str += ' ' + route['next_hop'] + if route.get('metric') is not None: + result_str += ' ' + str(route['metric']) + + for attribute, value in sorted(route.items()): + if attribute not in ('ip', 'next_hop', 'metric') and value is not None: + result_str += ' {0}={1}'.format(attribute, str(value).lower()) + + return result_str + @staticmethod def bool_to_string(boolean): if boolean: @@ -1657,6 +1762,20 @@ class Nmcli(object): return list return str + def get_route_params(self, raw_values): + routes_params = [] + for raw_value in raw_values: + route_params = {} + for parameter, value in re.findall(r'([\w-]*)\s?=\s?([^\s,}]*)', raw_value): + if parameter == 'nh': + route_params['next_hop'] = value + elif parameter == 'mt': + route_params['metric'] = value + else: + route_params[parameter] = value + routes_params.append(route_params) + return [self.route_to_string(route_params) for route_params in routes_params] + def list_connection_info(self): cmd = [self.nmcli_bin, '--fields', 'name', '--terse', 'con', 'show'] (rc, out, err) = self.execute_command(cmd) @@ -1852,13 +1971,7 @@ class Nmcli(object): if key in conn_info: current_value = conn_info[key] if key in ('ipv4.routes', 'ipv6.routes') and current_value is not None: - # ipv4.routes and ipv6.routes do not have same options and show_connection() format - # options: ['10.11.0.0/24 10.10.0.2', '10.12.0.0/24 10.10.0.2 200'] - # show_connection(): ['{ ip = 10.11.0.0/24, nh = 10.10.0.2 }', '{ ip = 10.12.0.0/24, nh = 10.10.0.2, mt = 200 }'] - # Need to convert in order to compare both - current_value = [re.sub(r'^{\s*ip\s*=\s*([^, ]+),\s*nh\s*=\s*([^} ]+),\s*mt\s*=\s*([^} ]+)\s*}', r'\1 \2 \3', - route) for route in current_value] - current_value = [re.sub(r'^{\s*ip\s*=\s*([^, ]+),\s*nh\s*=\s*([^} ]+)\s*}', r'\1 \2', route) for route in current_value] + current_value = self.get_route_params(current_value) if key == self.mac_setting: # MAC addresses are case insensitive, nmcli always reports them in uppercase value = value.upper() @@ -1942,6 +2055,18 @@ def main(): gw4=dict(type='str'), gw4_ignore_auto=dict(type='bool', default=False), routes4=dict(type='list', elements='str'), + routes4_extended=dict(type='list', + elements='dict', + options=dict( + ip=dict(type='str', required=True), + next_hop=dict(type='str'), + metric=dict(type='int'), + table=dict(type='int'), + tos=dict(type='int'), + cwnd=dict(type='int'), + mtu=dict(type='int'), + onlink=dict(type='bool') + )), route_metric4=dict(type='int'), routing_rules4=dict(type='list', elements='str'), never_default4=dict(type='bool', default=False), @@ -1958,6 +2083,17 @@ def main(): dns6_search=dict(type='list', elements='str'), dns6_ignore_auto=dict(type='bool', default=False), routes6=dict(type='list', elements='str'), + routes6_extended=dict(type='list', + elements='dict', + options=dict( + ip=dict(type='str', required=True), + next_hop=dict(type='str'), + metric=dict(type='int'), + table=dict(type='int'), + cwnd=dict(type='int'), + mtu=dict(type='int'), + onlink=dict(type='bool') + )), route_metric6=dict(type='int'), method6=dict(type='str', choices=['ignore', 'auto', 'dhcp', 'link-local', 'manual', 'shared', 'disabled']), ip_privacy6=dict(type='str', choices=['disabled', 'prefer-public-addr', 'prefer-temp-addr', 'unknown']), @@ -2014,7 +2150,9 @@ def main(): gsm=dict(type='dict'), wireguard=dict(type='dict'), ), - mutually_exclusive=[['never_default4', 'gw4']], + mutually_exclusive=[['never_default4', 'gw4'], + ['routes4_extended', 'routes4'], + ['routes6_extended', 'routes6']], required_if=[("type", "wifi", [("ssid")])], supports_check_mode=True, ) diff --git a/tests/unit/plugins/modules/net_tools/test_nmcli.py b/tests/unit/plugins/modules/net_tools/test_nmcli.py index 0f0e041fd8..546cae20e8 100644 --- a/tests/unit/plugins/modules/net_tools/test_nmcli.py +++ b/tests/unit/plugins/modules/net_tools/test_nmcli.py @@ -169,6 +169,17 @@ TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_ROUTE = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'ifname': 'ethernet_non_existant', + 'ip6': '2001:beef:cafe:10::1/64', + 'routes6_extended': [{'ip': 'fd2e:446f:d85d:5::/64', + 'next_hop': '2001:beef:cafe:10::2'}], + 'method6': 'manual', + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_ROUTE_SHOW_OUTPUT = """\ @@ -197,6 +208,14 @@ TESTCASE_ETHERNET_MOD_IPV4_INT_WITH_ROUTE_AND_METRIC = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'routes4_extended': [{'ip': '192.168.200.0/24', 'next_hop': '192.168.1.1'}], + 'route_metric4': 10, + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_MOD_IPV4_INT_WITH_ROUTE_AND_METRIC_SHOW_OUTPUT = """\ @@ -218,6 +237,14 @@ TESTCASE_ETHERNET_MOD_IPV6_INT_WITH_ROUTE_AND_METRIC = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'routes6_extended': [{'ip': 'fd2e:446f:d85d:5::/64', 'next_hop': '2001:beef:cafe:10::2'}], + 'route_metric6': 10, + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_MOD_IPV6_INT_WITH_ROUTE_AND_METRIC_SHOW_OUTPUT = """\ @@ -241,6 +268,17 @@ TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_MULTIPLE_ROUTES = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'ifname': 'ethernet_non_existant', + 'ip6': '2001:beef:cafe:10::1/64', + 'routes6_extended': [{'ip': 'fd2e:446f:d85d:5::/64', 'next_hop': '2001:beef:cafe:10::2'}, + {'ip': 'fd2e:8890:abcd:25::/64', 'next_hop': '2001:beef:cafe:10::5'}], + 'method6': 'manual', + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_MULTIPLE_ROUTES_SHOW_OUTPUT = """\ @@ -273,6 +311,18 @@ TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_ROUTE_AND_METRIC = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'ifname': 'ethernet_non_existant', + 'method4': 'disabled', + 'ip6': '2001:beef:cafe:10::1/64', + 'routes6_extended': [{'ip': 'fd2e:446f:d85d:5::/64', 'next_hop': '2001:beef:cafe:10::2'}], + 'route_metric6': 5, + 'method6': 'manual', + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_ROUTE_AND_METRIC_SHOW_OUTPUT = """\ @@ -305,6 +355,19 @@ TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_MULTIPLE_ROUTES_AND_METRIC = [ 'state': 'present', '_ansible_check_mode': False, }, + { + 'type': 'ethernet', + 'conn_name': 'non_existent_nw_device', + 'ifname': 'ethernet_non_existant', + 'method4': 'disabled', + 'ip6': '2001:beef:cafe:10::1/64', + 'routes6_extended': [{'ip': 'fd2e:446f:d85d:5::/64', 'next_hop': '2001:beef:cafe:10::2'}, + {'ip': 'fd2e:8890:abcd:25::/64', 'next_hop': '2001:beef:cafe:10::5'}], + 'route_metric6': 5, + 'method6': 'manual', + 'state': 'present', + '_ansible_check_mode': False, + }, ] TESTCASE_ETHERNET_ADD_IPV6_INT_WITH_MULTIPLE_ROUTES_AND_METRIC_SHOW_OUTPUT = """\