From 7c0579ddfc1dc9519df8df6b68df3ed05d27d4f6 Mon Sep 17 00:00:00 2001 From: Wen Liang Date: Mon, 13 Feb 2023 15:38:53 -0500 Subject: [PATCH] initscripts: Configure output device in routes Without an explicit output device, the kernel might use a different output device than intended by the user. Therefore, use the interface name of connections to specify it if it is available. Otherwise, educate the user about this potential problem with a warning. This aligns the behavior with NetworkManager which configures the output device in routes when activating profiles on devices. Fixes: https://bugzilla.redhat.com/2168735 Signed-off-by: Wen Liang --- library/network_connections.py | 13 ++ tests/playbooks/tests_route_device.yml | 188 +++++++++++++++++++++++ tests/tests_route_device_initscripts.yml | 16 ++ tests/tests_route_device_nm.yml | 20 +++ tests/unit/test_network_connections.py | 164 +++++++++++++++++++- 5 files changed, 398 insertions(+), 3 deletions(-) create mode 100644 tests/playbooks/tests_route_device.yml create mode 100644 tests/tests_route_device_initscripts.yml create mode 100644 tests/tests_route_device_nm.yml diff --git a/library/network_connections.py b/library/network_connections.py index 3e6eda95..1f9f6eb4 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -531,6 +531,19 @@ def ifcfg_create( line = r["network"] + "/" + str(r["prefix"]) if r["gateway"]: line += " via " + r["gateway"] + if connection["interface_name"]: + line += " dev " + connection["interface_name"] + else: + warn_fcn( + "The connection {0} does not specify an interface name. " + "Therefore, the route to {1}/{2} will be configured without " + "the output device and the kernel will choose it " + "automatically which might result in an unwanted device being " + "used. To avoid this, specify `interface_name` in the " + "connection appropriately.".format( + connection["name"], r["network"], r["prefix"] + ), + ) if r["metric"] != -1: line += " metric " + str(r["metric"]) diff --git a/tests/playbooks/tests_route_device.yml b/tests/playbooks/tests_route_device.yml new file mode 100644 index 00000000..bf5eeecd --- /dev/null +++ b/tests/playbooks/tests_route_device.yml @@ -0,0 +1,188 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Test output device of routes + hosts: all + vars: + type: veth + interface0: ethtest0 + interface1: ethtest1 + + tasks: + - name: Set type and interface0 + set_fact: + type: "{{ type }}" + interface: "{{ interface0 }}" + - name: Show interfaces + include_tasks: tasks/show_interfaces.yml + - name: Manage test interface + include_tasks: tasks/manage_test_interface.yml + vars: + state: present + - name: Assert device is present + include_tasks: tasks/assert_device_present.yml + - name: Set interface1 + set_fact: + interface: "{{ interface1 }}" + - name: Show interfaces + include_tasks: tasks/show_interfaces.yml + - name: Manage test interface + include_tasks: tasks/manage_test_interface.yml + vars: + state: present + - name: Assert device is present + include_tasks: tasks/assert_device_present.yml + - name: Test the route or the warning log when configuring the route with + or without the interface name + block: + - name: Configure the IP addresses and the route with interface name + specified + import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface0 }}" + interface_name: "{{ interface0 }}" + state: up + type: ethernet + autoconnect: false + ip: + address: + - 198.51.100.3/24 + - 2001:db8::2/32 + route: + - network: 198.51.10.64 + prefix: 26 + gateway: 198.51.100.6 + metric: 4 + - network: 2001:db6::4 + prefix: 128 + gateway: 2001:db8::1 + metric: 2 + - name: "{{ interface1 }}" + interface_name: "{{ interface1 }}" + state: up + type: ethernet + autoconnect: false + ip: + address: + - 198.51.100.6/24 + - 2001:db8::4/32 + route: + - network: 198.51.12.128 + prefix: 26 + gateway: 198.51.100.1 + metric: 2 + - name: Get the IPv4 routes from the route table main + command: ip -4 route + register: route_table_main_ipv4 + changed_when: false + + - name: Assert that the route table main contains the specified IPv4 + routes + assert: + that: + # When running with nm provider, the route will be configured + # with `proto static` + # In RHEL-6.10 managed host, the attribute in the route is not + # equally spaced + - route_table_main_ipv4.stdout is search("198.51.10.64/26 via + 198.51.100.6 dev ethtest0\s+(proto static )?metric 4") + - route_table_main_ipv4.stdout is search("198.51.12.128/26 via + 198.51.100.1 dev ethtest1\s+(proto static )?metric 2") + msg: "the route table main does not contain the specified IPv4 + route" + + - name: Get the IPv6 routes from the route table main + command: ip -6 route + register: route_table_main_ipv6 + changed_when: false + + - name: Assert that the route table main contains the specified IPv6 + routes + assert: + that: + - route_table_main_ipv6.stdout is search("2001:db6::4 via + 2001:db8::1 dev ethtest0\s+(proto static )?metric 2") + msg: "the route table main does not contain the specified IPv6 + route" + - name: Get the interface1 MAC address + command: cat /sys/class/net/{{ interface1 }}/address + register: interface1_mac + changed_when: false + - name: Configure the IP addresses and the route with only the MAC + address specified + import_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface1 }}" + mac: "{{ interface1_mac.stdout }}" + type: ethernet + autoconnect: false + ip: + address: + - 198.51.100.4/24 + - 2001:db8::6/32 + route: + - network: 198.58.10.64 + prefix: 26 + gateway: 198.51.100.102 + metric: 4 + + - name: Assert that the warning about specifying the route without + the output device is logged for initscripts provider + assert: + that: + - __network_connections_result.stderr is search("\[003\] + .0, state.None persistent_state.present, + '{{ interface1 }}'. The connection {{ interface1 }} does not + specify an interface name. Therefore, the route to + 198.58.10.64/26 will be configured without the output device + and the kernel will choose it automatically which might result + in an unwanted device being used. To avoid this, specify + `interface_name` in the connection appropriately.") + msg: The warning about specifying the route without the output + device is not logged for initscripts provider + when: network_provider == "initscripts" + + - name: Assert that no warning is logged for nm provider + assert: + that: + - __network_connections_result.stderr is not search("") + msg: The warning is logged for nm provider + when: network_provider == "nm" + always: + - name: Remove test configuration + block: + - name: Bring down test devices and profiles + include_role: + name: linux-system-roles.network + vars: + network_connections: + - name: "{{ interface0 }}" + persistent_state: absent + state: down + - name: "{{ interface1 }}" + persistent_state: absent + state: down + - name: Delete interface1 + include_tasks: tasks/delete_interface.yml + - name: Assert interface1 is absent + include_tasks: tasks/assert_device_absent.yml + - name: Set interface0 + set_fact: + interface: "{{ interface0 }}" + - name: Delete interface0 + include_tasks: tasks/delete_interface.yml + - name: Assert interface0 is absent + include_tasks: tasks/assert_device_absent.yml + - name: Assert interface0 profile and interface1 profile are absent + include_tasks: tasks/assert_profile_absent.yml + vars: + profile: "{{ item }}" + loop: + - "{{ interface0 }}" + - "{{ interface1 }}" + tags: + - "tests::cleanup" +... diff --git a/tests/tests_route_device_initscripts.yml b/tests/tests_route_device_initscripts.yml new file mode 100644 index 00000000..1f93fa43 --- /dev/null +++ b/tests/tests_route_device_initscripts.yml @@ -0,0 +1,16 @@ +# SPDX-License-Identifier: BSD-3-Clause +# This file was generated by ensure_provider_tests.py +--- +- hosts: all + name: Run playbook 'playbooks/tests_route_device.yml' with initscripts as provider + tasks: + - include_tasks: tasks/el_repo_setup.yml + - name: Set network provider to 'initscripts' + set_fact: + network_provider: initscripts + tags: + - always + +- import_playbook: playbooks/tests_route_device.yml + when: (ansible_distribution in ['CentOS','RedHat'] and + ansible_distribution_major_version | int < 9) diff --git a/tests/tests_route_device_nm.yml b/tests/tests_route_device_nm.yml new file mode 100644 index 00000000..e034c38b --- /dev/null +++ b/tests/tests_route_device_nm.yml @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: BSD-3-Clause +# This file was generated by ensure_provider_tests.py +--- +# set network provider and gather facts +- hosts: all + name: Run playbook 'playbooks/tests_route_device.yml' with nm as provider + tasks: + - include_tasks: tasks/el_repo_setup.yml + - name: Set network provider to 'nm' + set_fact: + network_provider: nm + tags: + - always + + +# The test requires or should run with NetworkManager, therefore it cannot run +# on RHEL/CentOS 6 +- import_playbook: playbooks/tests_route_device.yml + when: + - ansible_distribution_major_version != '6' diff --git a/tests/unit/test_network_connections.py b/tests/unit/test_network_connections.py index f55ff0e7..e53e33dc 100644 --- a/tests/unit/test_network_connections.py +++ b/tests/unit/test_network_connections.py @@ -2651,7 +2651,7 @@ def test_route_metric_prefix(self): "DEVICE": "555", }, "keys": None, - "route": "192.168.45.0/24 metric 545\n192.168.46.0/30\n", + "route": "192.168.45.0/24 dev 555 metric 545\n192.168.46.0/30 dev 555\n", "route6": None, "rule": None, "rule6": None, @@ -2772,7 +2772,7 @@ def test_route_v6(self): { "ifcfg": "", "keys": None, - "route": "192.168.40.0/24 metric 545\n192.168.46.0/30", + "route": "192.168.40.0/24 dev e556 metric 545\n192.168.46.0/30", "route6": "a:b:c:f::/64", "rule": None, "rule6": None, @@ -2792,13 +2792,171 @@ def test_route_v6(self): "DEVICE": "e556", }, "keys": None, - "route": "192.168.40.0/24 metric 545\n192.168.46.0/30\n" + "route": "192.168.40.0/24 dev e556 metric 545\n" + "192.168.46.0/30\n" + "192.168.45.0/24 dev e556 metric 545\n" + "192.168.46.0/30 dev e556\n", + "route6": "a:b:c:f::/64\na:b:c:d::/64 dev e556\n", + "rule": None, + "rule6": None, + } + ], + ) + + def test_route_without_interface_name(self): + route_device_warning = ( + "The connection e556 does not specify an interface name. Therefore, the " + "route to {0} will be configured without the output device and the kernel " + "will choose it automatically which might result in an unwanted device " + "being used. To avoid this, specify `interface_name` in the connection " + "appropriately." + ) + self.maxDiff = None + self.do_connections_validate( + [ + { + "actions": ["present", "up"], + "autoconnect": True, + "check_iface_exists": True, + "ethernet": ETHERNET_DEFAULTS, + "ethtool": ETHTOOL_DEFAULTS, + "force_state_change": None, + "ignore_errors": None, + "interface_name": None, + "ip": { + "gateway6": None, + "gateway4": None, + "route_metric4": None, + "auto6": True, + "ipv6_disabled": False, + "dhcp4": True, + "address": [], + "auto_gateway": None, + "route_append_only": True, + "rule_append_only": False, + "route": [ + { + "family": socket.AF_INET, + "network": "192.168.45.0", + "prefix": 24, + "gateway": None, + "metric": 545, + "table": None, + }, + { + "family": socket.AF_INET, + "network": "192.168.46.0", + "prefix": 30, + "gateway": None, + "metric": -1, + "table": None, + }, + { + "family": socket.AF_INET6, + "network": "a:b:c:d::", + "prefix": 64, + "gateway": None, + "metric": -1, + "table": None, + }, + ], + "routing_rule": [], + "dns": [], + "dns_options": [], + "dns_priority": 0, + "dns_search": ["aa", "bb"], + "route_metric6": None, + "dhcp4_send_hostname": None, + }, + "mac": "12:23:34:45:56:60", + "cloned_mac": "default", + "match": {}, + "controller": None, + "ieee802_1x": None, + "wireless": None, + "mtu": None, + "name": "e556", + "parent": None, + "persistent_state": "present", + "port_type": None, + "state": "up", + "type": "ethernet", + "wait": None, + "zone": "external", + } + ], + [ + { + "name": "e556", + "state": "up", + "type": "ethernet", + "zone": "external", + "mac": "12:23:34:45:56:60", + "ip": { + "dns_search": ["aa", "bb"], + "route_append_only": True, + "rule_append_only": False, + "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", + "HWADDR": "12:23:34:45:56:60", + "IPV6INIT": "yes", + "IPV6_AUTOCONF": "yes", + "NM_CONTROLLED": "no", + "ONBOOT": "yes", + "TYPE": "Ethernet", + "ZONE": "external", + }, + "keys": None, + "route": "192.168.40.0/24 metric 545\n" + "192.168.46.0/30\n" "192.168.45.0/24 metric 545\n", "route6": "a:b:c:f::/64\na:b:c:d::/64\n", "rule": None, "rule6": None, } ], + initscripts_expected_warnings=[ + route_device_warning.format("192.168.45.0/24"), + route_device_warning.format("192.168.46.0/30"), + route_device_warning.format("a:b:c:d::/64"), + ], ) def test_802_1x_1(self):