diff --git a/README.md b/README.md index 01ff7456..f0b26266 100644 --- a/README.md +++ b/README.md @@ -1472,6 +1472,21 @@ and `initscripts`). That means, you can use the same playbook with NetworkManage and initscripts. However, note that not every option is handled exactly the same by every provider. Do a test run first with `--check`. +With the `nm` provider, check mode describes the changes to existing connection +profiles in the role's log output. It lists individual additions and removals for +list properties such as static routes, IP addresses, and DNS servers, and shows +old and new values for properties such as MTU. Added and removed settings are +identified by name. For example: + +```text +[003] #0, state:up persistent_state:present, 'eth0': change property 802-3-ethernet.mtu from 1500 to 9000 +[004] #0, state:up persistent_state:present, 'eth0': remove DNS server 192.0.2.53 (ipv4.dns) +[005] #0, state:up persistent_state:present, 'eth0': add static route 192.168.100.0/24 via 192.168.1.129 (ipv4.routes) +``` + +Secret values, including passwords and private keys, are redacted. The details +are part of the `ansible-playbook --check` output and do not need `--diff`. + It is not supported to create a configuration for one provider, and expect another provider to handle them. For example, creating profiles with the `initscripts` provider, and later enabling NetworkManager is not guaranteed to work automatically. Possibly, diff --git a/library/network_connections.py b/library/network_connections.py index 03d994aa..349e7443 100644 --- a/library/network_connections.py +++ b/library/network_connections.py @@ -63,6 +63,7 @@ import errno import functools +import numbers import os import re import shlex @@ -899,28 +900,221 @@ def _cmp(a, b): cons.sort(cmp=_cmp) return cons + @staticmethod + def _connection_normalized_clone(con): + con = Util.NM().SimpleConnection.new_clone(con) + try: + con.normalize() + except Exception: + pass + return con + def connection_compare( self, con_a, con_b, normalize_a=False, normalize_b=False, compare_flags=None ): - NM = Util.NM() - if normalize_a: - con_a = NM.SimpleConnection.new_clone(con_a) - try: - con_a.normalize() - except Exception: - pass + con_a = self._connection_normalized_clone(con_a) if normalize_b: - con_b = NM.SimpleConnection.new_clone(con_b) - try: - con_b.normalize() - except Exception: - pass + con_b = self._connection_normalized_clone(con_b) if compare_flags is None: - compare_flags = NM.SettingCompareFlags.IGNORE_TIMESTAMP + compare_flags = Util.NM().SettingCompareFlags.IGNORE_TIMESTAMP return con_a.compare(con_b, compare_flags) + @staticmethod + def _setting_diff_values(setting): + NM = Util.NM() + values = {} + + def collect(setting, name, value, flags): + # NM_SETTING_PARAM_SECRET is 0x400; older typelibs expose it as 4. + secret = bool(flags & 0x400) + if setting.get_name() == "802-1x": + # libnm does not mark embedded keys as secrets. A PKCS#12 + # client certificate can contain a key, and PKCS#11 URIs a PIN. + if name in ("private-key", "phase2-private-key"): + secret = True + elif name in ( + "ca-cert", + "client-cert", + "phase2-ca-cert", + "phase2-client-cert", + ): + get_scheme = getattr( + setting, "get_%s_scheme" % name.replace("-", "_") + ) + scheme = get_scheme() + secret = ( + secret + or scheme == getattr(NM.Setting8021xCKScheme, "PKCS11", None) + or ( + name in ("client-cert", "phase2-client-cert") + and scheme == NM.Setting8021xCKScheme.BLOB + ) + ) + if secret: + value = None + elif setting.get_name() in ("ipv4", "ipv6") and name == "dns-options": + value = ( + setting.get_property(name) if setting.has_dns_options() else None + ) + elif setting.get_name() in ("ipv4", "ipv6") and name == "routing-rules": + value = [ + setting.get_routing_rule(i) + for i in range(setting.get_num_routing_rules()) + ] + elif setting.find_property(name) is not None: + # Enumeration loses the element types of boxed arrays. Property + # access preserves them; dynamic ethtool options have no pspec. + value = setting.get_property(name) + values[name] = (value, secret) + + setting.enumerate_values(collect) + return values + + @classmethod + def _setting_diff_value(cls, value): + NM = Util.NM() + GLib = Util.GLib() + if isinstance(value, GLib.Variant): + return cls._setting_diff_value(value.unpack()) + if isinstance(value, GLib.Bytes): + return repr(value.get_data()) + if isinstance(value, (NM.IPRoute, NM.IPAddress)): + if isinstance(value, NM.IPRoute): + result = "%s/%s" % (value.get_dest(), value.get_prefix()) + if value.get_next_hop(): + result += " via %s" % value.get_next_hop() + if value.get_metric() != -1: + result += " metric %s" % value.get_metric() + else: + result = "%s/%s" % (value.get_address(), value.get_prefix()) + for name in sorted(value.get_attribute_names()): + result += " %s=%s" % ( + name, + cls._setting_diff_value(value.get_attribute(name)), + ) + return result + if isinstance(value, getattr(NM, "IPRoutingRule", ())): + return value.to_string(NM.IPRoutingRuleAsStringFlags.NONE, None) + if isinstance(value, getattr(NM, "BridgeVlan", ())): + return value.to_str() + if isinstance(value, getattr(NM, "TCQdisc", ())): + return NM.utils_tc_qdisc_to_str(value) + if isinstance(value, getattr(NM, "TCTfilter", ())): + return NM.utils_tc_tfilter_to_str(value) + if isinstance(value, getattr(NM, "SriovVF", ())): + return NM.utils_sriov_vf_to_str(value, False) + if isinstance(value, (list, tuple)): + return "[%s]" % ", ".join(cls._setting_diff_value(v) for v in value) + if isinstance(value, dict): + return "{%s}" % ", ".join( + "%s: %s" % (cls._setting_diff_value(k), cls._setting_diff_value(v)) + for k, v in sorted(value.items()) + ) + if isinstance(value, numbers.Integral): + return str(value) + return repr(value) + + @classmethod + def _setting_diff_property(cls, setting_name, name, old, new): + property_name = "%s.%s" % (setting_name, name) + old_value, old_secret = old + new_value, new_secret = new + if old_secret or new_secret: + yield "change property %s from to " % property_name + return + + if isinstance(old_value, (list, tuple)) or isinstance(new_value, (list, tuple)): + label = "value" + if setting_name in ("ipv4", "ipv6"): + label = { + "routes": "static route", + "addresses": "IP address", + "dns": "DNS server", + "dns-search": "DNS search domain", + "dns-options": "DNS option", + "routing-rules": "routing rule", + }.get(name, label) + + def format_item(value): + if setting_name in ("ipv4", "ipv6") and name in ( + "dns", + "dns-search", + "dns-options", + ): + return value.replace("\n", "\\n").replace("\r", "\\r") + return cls._setting_diff_value(value) + + old_items = set(format_item(v) for v in (old_value or [])) + new_items = set(format_item(v) for v in (new_value or [])) + for value in sorted(old_items - new_items): + yield "remove %s %s (%s)" % (label, value, property_name) + for value in sorted(new_items - old_items): + yield "add %s %s (%s)" % (label, value, property_name) + if old_items != new_items: + return + + # Equal membership can still differ in order (for example, DNS servers). + yield "change property %s from %s to %s" % ( + property_name, + cls._setting_diff_value(old_value), + cls._setting_diff_value(new_value), + ) + + @staticmethod + def _connection_settings(con): + if hasattr(con, "get_settings"): + return dict((s.get_name(), s) for s in (con.get_settings() or [])) + + # Before libnm 1.10, settings are only exposed through their values. + settings = {} + + def collect(setting, name, value, flags): + settings[setting.get_name()] = setting + + con.for_each_setting_value(collect) + return settings + + def connection_diff( + self, con_a, con_b, normalize_a=False, normalize_b=False, compare_flags=None + ): + """Describe changes from con_a to con_b without modifying either profile.""" + if normalize_a: + con_a = self._connection_normalized_clone(con_a) + if normalize_b: + con_b = self._connection_normalized_clone(con_b) + if compare_flags is None: + compare_flags = Util.NM().SettingCompareFlags.IGNORE_TIMESTAMP + + settings_a = self._connection_settings(con_a) + settings_b = self._connection_settings(con_b) + for name in sorted(set(settings_a) | set(settings_b)): + setting_a = settings_a.get(name) + setting_b = settings_b.get(name) + # Introspection does not allow None as the other setting in diff(). + if setting_a is None: + yield "add setting %s" % name + setting_a = type(setting_b)() + elif setting_b is None: + yield "remove setting %s" % name + setting_b = type(setting_a)() + _same, differences = setting_a.diff(setting_b, compare_flags, False, {}) + if not differences: + continue + values_a = self._setting_diff_values(setting_a) + values_b = self._setting_diff_values(setting_b) + for property_name in sorted(differences): + # Python 2 does not support yield from. + # pylint: disable=unknown-option-value,use-yield-from + for message in self._setting_diff_property( + name, + property_name, + values_a.get(property_name, (None, False)), + values_b.get(property_name, (None, False)), + ): + yield message + def connection_is_active(self, con): NM = Util.NM() for ac in self.active_connection_list(connections=[con]): @@ -2505,7 +2699,15 @@ def run_action_present(self, idx): idx, "update connection %s, %s" % (con_cur.get_id(), con_cur.get_uuid()) ) self.connections_data_set_changed(idx) - if self.check_mode == CheckMode.REAL_RUN: + if self.check_mode == CheckMode.DRY_RUN: + try: + for message in self.nmutil.connection_diff( + con_cur, con_new, normalize_a=True + ): + self.log_info(idx, message) + except Exception as e: + self.log_warn(idx, "cannot describe connection changes: %s" % (e)) + elif self.check_mode == CheckMode.REAL_RUN: try: self.nmutil.connection_update(con_cur, con_new) except MyError as e: diff --git a/tests/ensure_provider_tests.py b/tests/ensure_provider_tests.py index 48c0e58c..48ce8358 100755 --- a/tests/ensure_provider_tests.py +++ b/tests/ensure_provider_tests.py @@ -114,6 +114,9 @@ }, "playbooks/tests_ignore_auto_dns.yml": {}, "playbooks/tests_bond_options.yml": {}, + "playbooks/tests_check_mode_diff.yml": { + MINIMUM_VERSION: "'1.8.0'", + }, "playbooks/tests_bond_port_match_by_mac.yml": {}, "playbooks/tests_eth_dns_support.yml": {}, "playbooks/tests_dummy.yml": {}, # wokeignore:rule=dummy diff --git a/tests/playbooks/tests_check_mode_diff.yml b/tests/playbooks/tests_check_mode_diff.yml new file mode 100644 index 00000000..ba5c7d07 --- /dev/null +++ b/tests/playbooks/tests_check_mode_diff.yml @@ -0,0 +1,198 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Test NetworkManager check mode diffs + hosts: all + vars: + network_provider: nm + interface: lsrchkdiff0 + profile: "{{ interface }}" + baseline_connections: + - name: "{{ interface }}" + type: dummy + state: up + autoconnect: false + mtu: 1500 + ip: + dhcp4: false + auto6: false + auto_gateway: false + address: + - 192.0.2.10/24 + - 2001:db8::10/64 + dns: + - 192.0.2.53 + - 2001:db8::53 + route: + - network: 203.0.113.0 + prefix: 24 + gateway: 192.0.2.1 + metric: 100 + proposed_connections: + - name: "{{ interface }}" + type: dummy + state: up + autoconnect: false + mtu: 9000 + ip: + dhcp4: false + auto6: false + auto_gateway: false + address: + - 192.0.2.20/24 + - 2001:db8::20/64 + dns: + - 192.0.2.54 + - 2001:db8::54 + route: + - network: 203.0.113.128 + prefix: 25 + gateway: 192.0.2.2 + metric: 200 + tasks: + - name: Initialize the network role + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: [] + + - name: Check that the test interface is absent + include_tasks: tasks/assert_device_absent.yml + + - name: List existing profiles + command: nmcli -g NAME connection show + changed_when: false + register: existing_profiles + + - name: Check that the test profile name is available + assert: + that: interface not in existing_profiles.stdout_lines + + - name: Test a managed connection in check mode + block: + - name: Create the baseline connection + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: "{{ baseline_connections }}" + + - name: Read the baseline profile + command: >- + nmcli -t -f connection,802-3-ethernet,ipv4,ipv6 + connection show {{ interface }} + changed_when: false + register: profile_before + + - name: Read the baseline addresses + command: ip -o address show dev {{ interface }} + changed_when: false + register: addresses_before + + - name: Preview the proposed connection changes + check_mode: true + block: + - name: Run the role in check mode + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: "{{ proposed_connections }}" + + - name: Verify the preview reports individual changes + assert: + that: + - __network_connections_result is changed + - item in __network_connections_result.stderr + fail_msg: "Missing change in check-mode output: {{ item }}" + loop: + - 'change property 802-3-ethernet.mtu from 1500 to 9000' + - 'remove IP address 192.0.2.10/24' + - 'add IP address 192.0.2.20/24' + - 'remove IP address 2001:db8::10/64' + - 'add IP address 2001:db8::20/64' + - 'remove DNS server 192.0.2.53' + - 'add DNS server 192.0.2.54' + - 'remove DNS server 2001:db8::53' + - 'add DNS server 2001:db8::54' + - 'remove static route 203.0.113.0/24 via 192.0.2.1 metric 100' + - 'add static route 203.0.113.128/25 via 192.0.2.2 metric 200' + + - name: Verify the preview uses numbered info log lines + assert: + that: + - >- + __network_connections_result.stderr_lines | + select('search', 'change property|add static route|remove static route|add IP address|remove IP address|add DNS server|remove DNS server') | + reject('match', '^\[[0-9]+\] \s+#0,') | list | length == 0 + + - name: Read the profile after check mode + command: >- + nmcli -t -f connection,802-3-ethernet,ipv4,ipv6 + connection show {{ interface }} + changed_when: false + register: profile_after + + - name: Read the addresses after check mode + command: ip -o address show dev {{ interface }} + changed_when: false + register: addresses_after + + - name: Verify check mode did not change the connection + vars: + before_addresses: >- + {{ addresses_before.stdout | + regex_findall('inet6? +([^ ]+)') | sort }} + after_addresses: >- + {{ addresses_after.stdout | + regex_findall('inet6? +([^ ]+)') | sort }} + assert: + that: + - profile_before.stdout == profile_after.stdout + - "'192.0.2.10/24' in before_addresses" + - "'2001:db8::10/64' in before_addresses" + - before_addresses == after_addresses + + - name: Preview the unchanged baseline connection + check_mode: true + block: + - name: Run the role with the baseline connection in check mode + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: "{{ baseline_connections }}" + + - name: Verify the baseline is unchanged + assert: + that: + - __network_connections_result is not changed + - "'change property' not in __network_connections_result.stderr" + + - name: Apply the proposed connection changes + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: "{{ proposed_connections }}" + + - name: Preview the applied connection + check_mode: true + block: + - name: Run the role with the applied connection in check mode + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: "{{ proposed_connections }}" + + - name: Verify the applied connection is unchanged + assert: + that: + - __network_connections_result is not changed + - "'change property' not in __network_connections_result.stderr" + always: + - name: Remove the test connection + tags: tests::cleanup + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + network_connections: + - name: "{{ interface }}" + state: down + persistent_state: absent + + - name: Verify the test profile was removed + tags: tests::cleanup + include_tasks: tasks/assert_profile_absent.yml + + - name: Verify the test interface was removed + tags: tests::cleanup + include_tasks: tasks/assert_device_absent.yml diff --git a/tests/tests_check_mode_diff_nm.yml b/tests/tests_check_mode_diff_nm.yml new file mode 100644 index 00000000..8494e220 --- /dev/null +++ b/tests/tests_check_mode_diff_nm.yml @@ -0,0 +1,53 @@ +# SPDX-License-Identifier: BSD-3-Clause +# This file was generated by ensure_provider_tests.py +--- +# set network provider and gather facts +# yamllint disable rule:line-length +- name: Run playbook 'playbooks/tests_check_mode_diff.yml' with nm as provider + hosts: all + tasks: + - name: Include the task 'el_repo_setup.yml' + include_tasks: tasks/el_repo_setup.yml + - name: Set network provider to 'nm' + set_fact: + network_provider: nm + tags: + - always + - name: Include distro variables + include_vars: vars/rh_distros_vars.yml + - name: Set platform facts + set_fact: + __network_distro_major_version: "{{ ansible_facts['distribution_major_version'] }}" + __network_is_rhel: "{{ ansible_facts['distribution'] == 'RedHat' }}" + __network_is_fedora: "{{ ansible_facts['distribution'] == 'Fedora' }}" + __network_is_centos: "{{ ansible_facts['distribution'] == 'CentOS' }}" + __network_is_os_family_rhel: "{{ ansible_facts['os_family'] == 'RedHat' }}" + __is_rh_distro: "{{ __network_is_rh_distro }}" + + - name: Install NetworkManager and get NetworkManager version + when: + - __network_distro_major_version != '6' + tags: + - always + block: + - name: Install NetworkManager + package: + name: NetworkManager + state: present + use: "{{ (__network_is_ostree | d(false)) | + ternary('ansible.posix.rhel_rpm_ostree', omit) }}" + - name: Get package info + package_facts: + - name: Get NetworkManager version + set_fact: + networkmanager_version: "{{ + ansible_facts.packages['NetworkManager'][0]['version'] }}" + + +# The test requires or should run with NetworkManager, therefore it cannot run +# on RHEL/CentOS 6 +- name: Import the playbook 'playbooks/tests_check_mode_diff.yml' + import_playbook: playbooks/tests_check_mode_diff.yml + when: + - __network_distro_major_version != '6' + - networkmanager_version is version('1.8.0', '>=') diff --git a/tests/unit/test_network_connections.py b/tests/unit/test_network_connections.py index 3f4ec489..4e24ed00 100644 --- a/tests/unit/test_network_connections.py +++ b/tests/unit/test_network_connections.py @@ -22,7 +22,15 @@ import network_lsr import network_lsr.argument_validator -from network_connections import IfcfgUtil, NMUtil, SysUtil, Util +from network_connections import ( + CheckMode, + Cmd_nm, + IfcfgUtil, + NMUtil, + RunEnvironmentAnsible, + SysUtil, + Util, +) from network_lsr.argument_validator import ValidationError try: @@ -4684,6 +4692,646 @@ def test_path_to_glib_bytes(self): self.assertEqual(result.get_data(), b"file:///my/test/path\x00") +class TestNMConnectionDiff(unittest.TestCase): + def setUp(self): + try: + self.nm = Util.NM() + except (ImportError, ValueError): + self.skipTest("no support for NM (libnm via pygobject)") + self.nmutil = NMUtil(nmclient=mock.Mock()) + + def connection(self): + con = self.nm.SimpleConnection.new() + setting = self.nm.SettingConnection.new() + setting.set_property("id", "diff-test") + setting.set_property("uuid", "deed76f5-97c5-4733-85f0-10922b0ed08b") + setting.set_property("type", "802-3-ethernet") + con.add_setting(setting) + wired = self.nm.SettingWired.new() + wired.set_property("mtu", 1500) + con.add_setting(wired) + for setting_type, family, address, dns in ( + (self.nm.SettingIP4Config, socket.AF_INET, "192.0.2.10", "192.0.2.53"), + (self.nm.SettingIP6Config, socket.AF_INET6, "2001:db8::10", "2001:db8::53"), + ): + setting = setting_type.new() + setting.set_property("method", "manual") + prefix = 24 if family == socket.AF_INET else 64 + setting.add_address(self.nm.IPAddress.new(family, address, prefix)) + setting.add_dns(dns) + con.add_setting(setting) + return con + + def route(self, metric=-1, table=None, route_type=None, src=None): + route = self.nm.IPRoute.new( + socket.AF_INET, "192.168.100.0", 24, "192.168.1.129", metric + ) + for name, variant_type, value in ( + ("table", "u", table), + ("type", "s", route_type), + ("src", "s", src), + ): + if value is not None: + route.set_attribute(name, Util.GLib().Variant(variant_type, value)) + return route + + def test_populated_settings_report_scalar_and_list_changes(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wired().set_property("mtu", 9000) + ip4 = proposed.get_setting_ip4_config() + ip4.add_route(self.route()) + ip4.remove_address(0) + ip4.add_address(self.nm.IPAddress.new(socket.AF_INET, "192.0.2.1", 24)) + ip4.remove_dns(0) + ip4.add_dns("192.0.2.54") + + lines = list(self.nmutil.connection_diff(current, proposed)) + + for line in ( + "change property 802-3-ethernet.mtu from 1500 to 9000", + "add static route 192.168.100.0/24 via 192.168.1.129 (ipv4.routes)", + "remove IP address 192.0.2.10/24 (ipv4.addresses)", + "add IP address 192.0.2.1/24 (ipv4.addresses)", + "remove DNS server 192.0.2.53 (ipv4.dns)", + "add DNS server 192.0.2.54 (ipv4.dns)", + ): + self.assertIn(line, lines) + self.assertFalse(any("setting " in line for line in lines)) + self.assertFalse(any("ipv6" in line for line in lines)) + + def test_route_metric_and_attributes_are_part_of_identity(self): + for original, replacement, old_detail, new_detail in ( + (self.route(10), self.route(20), "metric 10", "metric 20"), + (self.route(table=100), self.route(table=200), "table=100", "table=200"), + ( + self.route(route_type="unicast"), + self.route(route_type="blackhole"), + "type='unicast'", + "type='blackhole'", + ), + ( + self.route(src="192.0.2.1"), + self.route(src="192.0.2.2"), + "src='192.0.2.1'", + "src='192.0.2.2'", + ), + ): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + current.get_setting_ip4_config().add_route(original) + proposed.get_setting_ip4_config().add_route(replacement) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertIn( + "remove static route 192.168.100.0/24 " + "via 192.168.1.129 %s (ipv4.routes)" % old_detail, + lines, + ) + self.assertIn( + "add static route 192.168.100.0/24 via 192.168.1.129 %s (ipv4.routes)" + % new_detail, + lines, + ) + + def test_ipv6_and_direct_route_changes(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + current.get_setting_ip6_config().add_route( + self.nm.IPRoute.new(socket.AF_INET6, "2001:db8:1::", 64, None, -1) + ) + ip6 = proposed.get_setting_ip6_config() + ip6.add_route( + self.nm.IPRoute.new(socket.AF_INET6, "2001:db8:2::", 64, "2001:db8::1", 20) + ) + ip6.remove_address(0) + ip6.remove_dns(0) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + for line in ( + "remove static route 2001:db8:1::/64 (ipv6.routes)", + "add static route 2001:db8:2::/64 via 2001:db8::1 metric 20 (ipv6.routes)", + "remove IP address 2001:db8::10/64 (ipv6.addresses)", + "remove DNS server 2001:db8::53 (ipv6.dns)", + ): + self.assertIn(line, lines) + + def test_identical_boxed_values_do_not_produce_changes(self): + current = self.connection() + current.get_setting_ip4_config().add_route(self.route(10, table=100)) + proposed = self.nm.SimpleConnection.new_clone(current) + + self.assertEqual(list(self.nmutil.connection_diff(current, proposed)), []) + + def test_bridge_vlan_changes_preserve_unchanged_entries(self): + if not hasattr(self.nm, "BridgeVlan"): + self.skipTest("bridge VLANs are unavailable") + current = self.connection() + bridge = self.nm.SettingBridge.new() + bridge.add_vlan(self.nm.BridgeVlan.new(100, 100)) + vlan = self.nm.BridgeVlan.new(200, 200) + vlan.set_pvid(True) + vlan.set_untagged(True) + bridge.add_vlan(vlan) + current.add_setting(bridge) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_bridge().remove_vlan(1) + proposed.get_setting_bridge().add_vlan(self.nm.BridgeVlan.new(300, 310)) + + self.assertEqual( + list(self.nmutil.connection_diff(current, proposed)), + [ + "remove value 200 pvid untagged (bridge.vlans)", + "add value 300-310 (bridge.vlans)", + ], + ) + + def test_tc_changes_preserve_unchanged_entries(self): + if not hasattr(self.nm, "SettingTCConfig"): + self.skipTest("traffic control settings are unavailable") + current = self.connection() + tc = self.nm.SettingTCConfig.new() + for description in ("parent 1: handle 2: sfq", "root handle 1: fq_codel"): + tc.add_qdisc(self.nm.utils_tc_qdisc_from_str(description)) + tc.add_tfilter( + self.nm.utils_tc_tfilter_from_str( + "parent ffff: matchall action simple sdata hello" + ) + ) + current.add_setting(tc) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed_tc = proposed.get_setting(self.nm.SettingTCConfig) + proposed_tc.remove_qdisc(1) + proposed_tc.add_qdisc(self.nm.utils_tc_qdisc_from_str("root handle 1: fq")) + proposed_tc.remove_tfilter(0) + + self.assertEqual( + list(self.nmutil.connection_diff(current, proposed)), + [ + "remove value root handle 1: fq_codel (tc.qdiscs)", + "add value root handle 1: fq (tc.qdiscs)", + "remove value parent ffff: matchall action simple sdata hello " + "(tc.tfilters)", + ], + ) + + def test_sriov_changes_preserve_unchanged_entries(self): + if not hasattr(self.nm, "SettingSriov"): + self.skipTest("SR-IOV settings are unavailable") + current = self.connection() + sriov = self.nm.SettingSriov.new() + sriov.add_vf(self.nm.SriovVF.new(0)) + vf = self.nm.SriovVF.new(1) + vf.set_attribute("mac", Util.GLib().Variant("s", "02:00:00:00:00:01")) + sriov.add_vf(vf) + current.add_setting(sriov) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting(self.nm.SettingSriov).remove_vf(1) + + self.assertEqual( + list(self.nmutil.connection_diff(current, proposed)), + ["remove value 1 mac=02:00:00:00:00:01 (sriov.vfs)"], + ) + + def test_empty_dns_options_differ_from_unset(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_ip4_config().set_property("dns-options", []) + proposed.get_setting_ip6_config().set_property("dns-options", []) + + self.assertEqual( + list(self.nmutil.connection_diff(current, proposed)), + [ + "change property ipv4.dns-options from None to []", + "change property ipv6.dns-options from None to []", + ], + ) + self.assertEqual( + list(self.nmutil.connection_diff(proposed, current)), + [ + "change property ipv4.dns-options from [] to None", + "change property ipv6.dns-options from [] to None", + ], + ) + + def test_settings_without_get_settings_api(self): + class LegacyConnection(object): + def __init__(self, connection): + self.for_each_setting_value = connection.for_each_setting_value + + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wired().set_property("mtu", 9000) + proposed.add_setting(self.nm.SettingBridge.new()) + + self.assertEqual( + list( + self.nmutil.connection_diff( + LegacyConnection(current), LegacyConnection(proposed) + ) + ), + [ + "change property 802-3-ethernet.mtu from 1500 to 9000", + "add setting bridge", + ], + ) + + def test_list_order_changes_are_visible(self): + current = self.connection() + current.get_setting_ip4_config().add_dns("192.0.2.54") + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_ip4_config().clear_dns() + proposed.get_setting_ip4_config().add_dns("192.0.2.54") + proposed.get_setting_ip4_config().add_dns("192.0.2.53") + + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertTrue(any("ipv4.dns" in line for line in lines)) + self.assertFalse(any("add DNS server" in line for line in lines)) + self.assertFalse(any("remove DNS server" in line for line in lines)) + + def test_settings_added_and_removed_with_details(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + current.remove_setting(self.nm.SettingIP4Config) + proposed.remove_setting(self.nm.SettingIP6Config) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + for line in ( + "add setting ipv4", + "remove setting ipv6", + "add IP address 192.0.2.10/24 (ipv4.addresses)", + "remove IP address 2001:db8::10/64 (ipv6.addresses)", + "add DNS server 192.0.2.53 (ipv4.dns)", + "remove DNS server 2001:db8::53 (ipv6.dns)", + ): + self.assertIn(line, lines) + + def test_default_only_setting_is_reported(self): + setting_types = [self.nm.SettingWired] + if hasattr(self.nm, "SettingEthtool"): + setting_types.append(self.nm.SettingEthtool) + for setting_type in setting_types: + current = self.nm.SimpleConnection.new() + proposed = self.nm.SimpleConnection.new() + setting = setting_type.new() + proposed.add_setting(setting) + + self.assertIn( + "add setting %s" % setting.get_name(), + list(self.nmutil.connection_diff(current, proposed)), + ) + self.assertIn( + "remove setting %s" % setting.get_name(), + list(self.nmutil.connection_diff(proposed, current)), + ) + + def test_scalar_strings_boolean_and_unset(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_connection().set_property("id", "renamed") + proposed.get_setting_connection().set_property("autoconnect", False) + proposed.get_setting_ip4_config().set_property("gateway", "192.0.2.1") + + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertIn( + "change property connection.id from 'diff-test' to 'renamed'", lines + ) + self.assertIn( + "change property connection.autoconnect from True to False", lines + ) + self.assertTrue( + any("ipv4.gateway" in line and "'192.0.2.1'" in line for line in lines) + ) + + def test_normalization_and_timestamp_comparison_match(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.normalize() + proposed.get_setting_connection().set_property("timestamp", 2**64 - 1) + + self.assertTrue( + self.nmutil.connection_compare(current, proposed, normalize_a=True) + ) + self.assertEqual( + list(self.nmutil.connection_diff(current, proposed, normalize_a=True)), [] + ) + self.assertIn( + "change property connection.timestamp from 0 to 18446744073709551615", + list( + self.nmutil.connection_diff( + current, + proposed, + normalize_a=True, + compare_flags=self.nm.SettingCompareFlags.EXACT, + ) + ), + ) + self.assertEqual(current.get_setting_connection().get_timestamp(), 0) + + def test_secret_flag_from_legacy_introspection(self): + current = self.connection() + security = self.nm.SettingWirelessSecurity.new() + security.set_property("psk", "old-test-secret") + current.add_setting(security) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wired().set_property("mtu", 9000) + proposed.get_setting_wireless_security().set_property("psk", "new-test-secret") + + with mock.patch.object(self.nm, "SETTING_PARAM_SECRET", 4): + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertIn("change property 802-3-ethernet.mtu from 1500 to 9000", lines) + self.assertIn( + "change property 802-11-wireless-security.psk " + "from to ", + lines, + ) + self.assertNotIn("old-test-secret", "\n".join(lines)) + self.assertNotIn("new-test-secret", "\n".join(lines)) + + def test_secret_and_private_key_values_are_redacted(self): + current = self.connection() + security = self.nm.SettingWirelessSecurity.new() + security.set_property("psk", "old-test-secret") + current.add_setting(security) + authentication = self.nm.Setting8021x.new() + authentication.set_property("password", "old-eap-secret") + authentication.set_property( + "private-key", Util.GLib().Bytes.new(b"old-private-key-material") + ) + for property_name in ("client-cert", "phase2-client-cert"): + authentication.set_property( + property_name, Util.GLib().Bytes.new(b"old-pkcs12-private-key-material") + ) + current.add_setting(authentication) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wireless_security().set_property("psk", "new-test-secret") + proposed.get_setting_802_1x().set_property("password", "new-eap-secret") + proposed.get_setting_802_1x().set_property( + "private-key", Util.GLib().Bytes.new(b"new-private-key-material") + ) + for property_name in ("client-cert", "phase2-client-cert"): + proposed.get_setting_802_1x().set_property( + property_name, Util.GLib().Bytes.new(b"new-pkcs12-private-key-material") + ) + + lines = list(self.nmutil.connection_diff(current, proposed)) + output = "\n".join(lines) + + for property_name in ( + "802-11-wireless-security.psk", + "802-1x.password", + "802-1x.private-key", + "802-1x.client-cert", + "802-1x.phase2-client-cert", + ): + self.assertTrue( + any(property_name in line and "" in line for line in lines) + ) + for secret in ( + "old-test-secret", + "new-test-secret", + "old-eap-secret", + "new-eap-secret", + "old-private-key-material", + "new-private-key-material", + "old-pkcs12-private-key-material", + "new-pkcs12-private-key-material", + ): + self.assertNotIn(secret, output) + + def test_certificate_file_paths_remain_visible(self): + current = self.connection() + authentication = self.nm.Setting8021x.new() + properties = ("ca-cert", "client-cert", "phase2-ca-cert", "phase2-client-cert") + for property_name in properties: + authentication.set_property( + property_name, Util.path_to_glib_bytes("/old-client-cert.pem") + ) + current.add_setting(authentication) + proposed = self.nm.SimpleConnection.new_clone(current) + for property_name in properties: + proposed.get_setting_802_1x().set_property( + property_name, Util.path_to_glib_bytes("/new-client-cert.pem") + ) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + for property_name in properties: + self.assertTrue( + any( + "802-1x.%s" % property_name in line + and "/old-client-cert.pem" in line + and "/new-client-cert.pem" in line + for line in lines + ) + ) + self.assertNotIn("", "\n".join(lines)) + + def test_pkcs11_certificate_pins_are_redacted_in_check_mode(self): + if not hasattr(self.nm.Setting8021xCKScheme, "PKCS11"): + self.skipTest("PKCS#11 certificate URIs are unavailable") + current = self.connection() + authentication = self.nm.Setting8021x.new() + authentication.set_property("eap", ["tls"]) + authentication.set_property("identity", "diff-test") + authentication.set_property( + "private-key", Util.path_to_glib_bytes("/private-key.pem") + ) + current.add_setting(authentication) + proposed = self.nm.SimpleConnection.new_clone(current) + properties = ("ca-cert", "client-cert", "phase2-ca-cert", "phase2-client-cert") + for connection, pin in ( + (current, "old-uri-secret"), + (proposed, "new-uri-secret"), + ): + for property_name in properties: + uri = ( + "pkcs11:token=diff-test;object=cert;type=cert?pin-value=%s\x00" + % pin + ) + connection.get_setting_802_1x().set_property( + property_name, Util.GLib().Bytes.new(uri.encode("utf-8")) + ) + connection.normalize() + cmd = self.command(current, proposed, CheckMode.DRY_RUN) + + cmd.run_action_present(0) + + stderr = cmd.run_env._complete_kwargs(cmd.connections, {})["stderr"] + for property_name in properties: + self.assertIn( + "change property 802-1x.%s from to " + % property_name, + stderr, + ) + self.assertNotIn("old-uri-secret", stderr) + self.assertNotIn("new-uri-secret", stderr) + self.assertNotIn("pin-value", stderr) + self.assertEqual(cmd._nmutil.connection_update.call_count, 0) + + def test_dynamic_ethtool_option(self): + if not hasattr(self.nm, "SettingEthtool"): + self.skipTest("ethtool settings require NetworkManager 1.14") + current = self.connection() + ethtool = self.nm.SettingEthtool.new() + if not hasattr(ethtool, "option_set"): + self.skipTest("generic ethtool option API is unavailable") + ethtool.option_set("feature-gro", Util.GLib().Variant("b", True)) + current.add_setting(ethtool) + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting(self.nm.SettingEthtool).option_set( + "feature-gro", Util.GLib().Variant("b", False) + ) + + self.assertIn( + "change property ethtool.feature-gro from True to False", + list(self.nmutil.connection_diff(current, proposed)), + ) + + def test_routing_rules_use_readable_values(self): + if not hasattr(self.nm, "IPRoutingRule"): + self.skipTest("routing rules are unavailable") + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + for connection, priority in ((current, 100), (proposed, 200)): + rule = self.nm.IPRoutingRule.new(socket.AF_INET) + rule.set_priority(priority) + rule.set_from("192.0.2.0", 24) + rule.set_table(100) + connection.get_setting_ip4_config().add_routing_rule(rule) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertTrue( + any( + line.startswith("remove routing rule priority 100 ") + and "192.0.2.0/24" in line + and line.endswith("(ipv4.routing-rules)") + for line in lines + ) + ) + self.assertTrue( + any( + line.startswith("add routing rule priority 200 ") + and "192.0.2.0/24" in line + and line.endswith("(ipv4.routing-rules)") + for line in lines + ) + ) + self.assertNotIn("PtrArray", "\n".join(lines)) + + def test_added_secret_setting_is_redacted(self): + current = self.connection() + proposed = self.nm.SimpleConnection.new_clone(current) + security = self.nm.SettingWirelessSecurity.new() + security.set_property("psk", "new-test-secret") + proposed.add_setting(security) + + lines = list(self.nmutil.connection_diff(current, proposed)) + + self.assertIn("add setting 802-11-wireless-security", lines) + self.assertTrue(any("psk" in line and "" in line for line in lines)) + self.assertNotIn("new-test-secret", "\n".join(lines)) + + def command(self, current, proposed, check_mode): + run_env = RunEnvironmentAnsible() + run_env.module.params = {"__debug_flags": ""} + run_env._run_results_push(1) + cmd = Cmd_nm( + run_env=run_env, + connections_unvalidated=[], + connection_validator=mock.Mock(), + ) + cmd._check_mode = check_mode + cmd._connections = [ + { + "name": "diff-test", + "nm.uuid": current.get_uuid(), + "type": "ethernet", + "state": "up", + "persistent_state": "present", + "ignore_errors": None, + "ieee802_1x": None, + } + ] + cmd._nmutil = NMUtil(nmclient=mock.Mock()) + cmd._nmutil.connection_list = mock.Mock(return_value=[current]) + cmd._nmutil.connection_create = mock.Mock(return_value=proposed) + cmd._nmutil.connection_update = mock.Mock() + cmd._nm_provider = mock.Mock() + cmd._nm_provider.get_connections.return_value = [] + return cmd + + def test_unchanged_check_mode_does_not_report_diff(self): + current = self.connection() + current.normalize() + proposed = self.nm.SimpleConnection.new_clone(current) + cmd = self.command(current, proposed, CheckMode.DRY_RUN) + with mock.patch.object(cmd._nmutil, "connection_diff") as diff: + cmd.run_action_present(0) + + self.assertEqual(diff.call_count, 0) + self.assertEqual(cmd._nmutil.connection_update.call_count, 0) + self.assertFalse(cmd.is_changed_modified_system) + + def test_check_mode_diff_failure_only_warns(self): + current = self.connection() + current.normalize() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wired().set_property("mtu", 9000) + cmd = self.command(current, proposed, CheckMode.DRY_RUN) + + with mock.patch.object( + cmd._nmutil, "connection_diff", side_effect=RuntimeError("boom") + ): + cmd.run_action_present(0) + + stderr = cmd.run_env._complete_kwargs(cmd.connections, {})["stderr"] + self.assertIn("", stderr) + + def test_check_mode_logs_details_without_updating(self): + current = self.connection() + current.normalize() + proposed = self.nm.SimpleConnection.new_clone(current) + proposed.get_setting_wired().set_property("mtu", 9000) + for check_mode in (CheckMode.DRY_RUN, CheckMode.PRE_RUN, CheckMode.REAL_RUN): + cmd = self.command(current, proposed, check_mode) + + with mock.patch.object( + cmd._nmutil, "connection_diff", wraps=cmd._nmutil.connection_diff + ) as diff: + cmd.run_action_present(0) + + stderr = cmd.run_env._complete_kwargs(cmd.connections, {})["stderr"] + if check_mode == CheckMode.DRY_RUN: + self.assertEqual(diff.call_count, 1) + self.assertIn( + "change property 802-3-ethernet.mtu from 1500 to 9000", stderr + ) + self.assertTrue( + any( + line.startswith("[002] ") + and "#0, state:up persistent_state:present, 'diff-test':" + in line + for line in stderr.splitlines() + ) + ) + self.assertTrue(cmd.is_changed_modified_system) + else: + self.assertEqual(diff.call_count, 0) + self.assertNotIn("change property", stderr) + self.assertEqual( + cmd._nmutil.connection_update.call_count, + 1 if check_mode == CheckMode.REAL_RUN else 0, + ) + + class TestValidatorMatch(Python26CompatTestCase): def setUp(self): self.test_profile = {