diff --git a/README.md b/README.md index f6eade5ea..502a8a5c2 100644 --- a/README.md +++ b/README.md @@ -1540,6 +1540,15 @@ Routing rules and named routing tables are not supported when using the silently ignored. Use `network_provider: nm` (NetworkManager) for routing rule support. +Configuring `dns-resolver` through `network_state` is not supported when +NetworkManager has a `[global-dns]` or `[global-dns-domain-*]` section in its +configuration files (`/etc/NetworkManager/NetworkManager.conf` or a `conf.d` +snippet). nmstate applies DNS through NetworkManager's D-Bus global DNS API, and +NetworkManager rejects that while global DNS is set in a configuration file. The +role fails with an error naming the file. Either remove the section and reload +NetworkManager, or configure DNS on the connection profiles instead of +`dns-resolver`. + ### Handling potential problems When something goes wrong while configuring networking remotely, you might need diff --git a/library/network_state.py b/library/network_state.py index 1a449107d..09b70a999 100644 --- a/library/network_state.py +++ b/library/network_state.py @@ -42,6 +42,9 @@ returned: always """ +import glob +import os +import re import traceback from ansible.module_utils.basic import AnsibleModule, missing_required_lib @@ -55,6 +58,33 @@ NETWORK_HAS_NMSTATE = True NETWORK_NMSTATE_IMPORT_ERROR = None +# NetworkManager.conf(5) load order: a later dir shadows a same-named file. +# D-Bus-set global DNS lives in [.intern.*] groups and must not match. +NM_CONFIG_FILE = "/etc/NetworkManager/NetworkManager.conf" +NM_CONFIG_DIRS = [ + "/usr/lib/NetworkManager/conf.d", + "/run/NetworkManager/conf.d", + "/etc/NetworkManager/conf.d", +] +NM_GLOBAL_DNS_SECTION_RE = re.compile(b"^\\s*\\[global-dns(-domain-[^\\]]*)?\\]") + + +def find_nm_global_dns_config(): + """Return the NetworkManager config files that define [global-dns*] sections.""" + snippets = {} + for config_dir in NM_CONFIG_DIRS: + for path in glob.glob(os.path.join(config_dir, "*.conf")): + snippets[os.path.basename(path)] = path + found = [] + for path in [NM_CONFIG_FILE] + [snippets[name] for name in sorted(snippets)]: + try: + with open(path, "rb") as conf: + if any(NM_GLOBAL_DNS_SECTION_RE.match(line) for line in conf): + found.append(path) + except (IOError, OSError): + continue + return found + class NetworkState: def __init__(self, module, module_name): @@ -65,7 +95,21 @@ def __init__(self, module, module_name): self.previous_state = self.get_state_config() def run(self): + """Apply desired_state through nmstate and exit the module.""" desired_state = self.params["desired_state"] + # NetworkManager rejects nmstate's D-Bus global DNS writes while a config + # file defines global DNS, and nmstate reports that as an internal error. + if "dns-resolver" in desired_state: + global_dns_files = find_nm_global_dns_config() + if global_dns_files: + self.module.fail_json( + msg="Managing `dns-resolver` with `network_state` is not " + "supported while NetworkManager has a [global-dns] or " + "[global-dns-domain-*] section in its configuration (%s). " + "Remove the section and reload NetworkManager, or set the " + "DNS options on the connection profiles instead." + % ", ".join(global_dns_files) + ) libnmstate.apply(desired_state) current_state = self.get_state_config() if current_state != self.previous_state: diff --git a/tests/ensure_provider_tests.py b/tests/ensure_provider_tests.py index c7de7f5cb..48c0e58c7 100755 --- a/tests/ensure_provider_tests.py +++ b/tests/ensure_provider_tests.py @@ -141,6 +141,9 @@ "playbooks/tests_network_state.yml": { EXTRA_RUN_CONDITION: "__network_distro_major_version | int > 7", }, + "playbooks/tests_network_state_global_dns.yml": { + EXTRA_RUN_CONDITION: "__network_distro_major_version | int > 7", + }, "playbooks/tests_reapply.yml": {}, "playbooks/tests_route_table.yml": {}, "playbooks/tests_route_type.yml": { diff --git a/tests/playbooks/tests_network_state_global_dns.yml b/tests/playbooks/tests_network_state_global_dns.yml new file mode 100644 index 000000000..5ead82219 --- /dev/null +++ b/tests/playbooks/tests_network_state_global_dns.yml @@ -0,0 +1,59 @@ +# SPDX-License-Identifier: BSD-3-Clause +--- +- name: Play for rejecting dns-resolver in network_state when NetworkManager + has global-dns configured + hosts: all + vars: + global_dns_conf: /etc/NetworkManager/conf.d/99-lsr-test-global-dns.conf + tasks: + - name: Test network_state dns-resolver with NetworkManager global-dns + block: + - name: Configure global-dns in NetworkManager + copy: + dest: "{{ global_dns_conf }}" + content: | + [global-dns] + options=no-aaaa + mode: "0644" + + - name: Reload the NetworkManager configuration + command: nmcli general reload conf + changed_when: true + + - name: Configure the DNS with dns-resolver + include_tasks: tasks/run_role_with_clear_facts.yml + vars: + __sr_failed_when: false + network_state: + dns-resolver: + config: + server: + - 192.168.122.1 + + - name: Assert that the role reports global-dns as unsupported + assert: + that: + - __network_state_result is defined + - __network_state_result is failed + - __network_state_result.msg is search("global-dns") + msg: the role did not report the NetworkManager global-dns + configuration as unsupported + + always: + - name: Remove global-dns from NetworkManager + file: + path: "{{ global_dns_conf }}" + state: absent + tags: + - "tests::cleanup" + + - name: Reload the NetworkManager configuration after cleanup + command: nmcli general reload conf + changed_when: true + tags: + - "tests::cleanup" + + - name: Verify network state restored to default + include_tasks: tasks/check_network_dns.yml + tags: + - "tests::cleanup" diff --git a/tests/tests_network_state_global_dns_nm.yml b/tests/tests_network_state_global_dns_nm.yml new file mode 100644 index 000000000..b5392b1a4 --- /dev/null +++ b/tests/tests_network_state_global_dns_nm.yml @@ -0,0 +1,34 @@ +# 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_network_state_global_dns.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 }}" + + +# The test requires or should run with NetworkManager, therefore it cannot run +# on RHEL/CentOS 6 +- name: Import the playbook 'playbooks/tests_network_state_global_dns.yml' + import_playbook: playbooks/tests_network_state_global_dns.yml + when: + - __network_distro_major_version != '6' + - __network_distro_major_version | int > 7 diff --git a/tests/unit/test_network_state.py b/tests/unit/test_network_state.py new file mode 100644 index 000000000..6528d1c3e --- /dev/null +++ b/tests/unit/test_network_state.py @@ -0,0 +1,142 @@ +# -*- coding: utf-8 -*- +# SPDX-License-Identifier: BSD-3-Clause +"""Unit tests for network_state module helpers.""" + +from __future__ import absolute_import, division, print_function + +__metaclass__ = type + +import os +import shutil +import sys +import tempfile +import unittest + +try: + from unittest import mock +except ImportError: # py2 + import mock + +sys.modules["ansible.module_utils.basic"] = mock.Mock() +sys.modules["libnmstate"] = mock.Mock() + +# pylint: disable=import-error, wrong-import-position + +import network_state + + +class _FailJson(Exception): + """Raised by the mocked fail_json to stop the module.""" + + +class TestNmGlobalDnsConfig(unittest.TestCase): + """Tests for the NetworkManager global DNS config check.""" + + def setUp(self): + """Point the module at a temporary NetworkManager config tree.""" + self.root = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.root) + self.config_file = os.path.join(self.root, "NetworkManager.conf") + self.dirs = [os.path.join(self.root, d) for d in ("lib", "run", "etc")] + for d in self.dirs: + os.mkdir(d) + self._write(self.config_file, "[main]\nplugins=keyfile\n") + for name, value in ( + ("NM_CONFIG_FILE", self.config_file), + ("NM_CONFIG_DIRS", self.dirs), + ): + patcher = mock.patch.object(network_state, name, value) + patcher.start() + self.addCleanup(patcher.stop) + self.libnmstate = network_state.libnmstate + self.libnmstate.reset_mock() + self.module = mock.Mock() + self.module.fail_json.side_effect = _FailJson + + def _write(self, path, content, mode="w"): + """Write content to path.""" + with open(path, mode) as f: + f.write(content) + + def _find(self): + """Run find_nm_global_dns_config against the temporary tree.""" + return network_state.find_nm_global_dns_config() + + def _run(self, desired_state): + """Run NetworkState.run with desired_state and the mocked module.""" + self.module.params = {"desired_state": desired_state} + network_state.NetworkState(self.module, "network_state").run() + + def test_no_global_dns(self): + """No files match without a global-dns section.""" + self._write(os.path.join(self.dirs[2], "10-logging.conf"), "[logging]\n") + self.assertEqual(self._find(), []) + + def test_global_dns_in_main_file(self): + """NetworkManager.conf itself is reported.""" + self._write(self.config_file, "[main]\n[global-dns]\noptions=no-aaaa\n") + self.assertEqual(self._find(), [self.config_file]) + + def test_global_dns_in_snippet(self): + """A conf.d snippet is reported.""" + path = os.path.join(self.dirs[2], "90-dns-servers.conf") + self._write(path, "[global-dns]\noptions=no-aaaa\n") + self.assertEqual(self._find(), [path]) + + def test_global_dns_domain_section(self): + """global-dns-domain-* sections count as global DNS.""" + path = os.path.join(self.dirs[1], "50-dns.conf") + self._write(path, "[global-dns-domain-example.com]\nservers=192.0.2.1\n") + self.assertEqual(self._find(), [path]) + + def test_similar_section_name_ignored(self): + """Sections merely starting with global-dns do not match.""" + self._write(os.path.join(self.dirs[2], "x.conf"), "[global-dnsfoo]\n") + self.assertEqual(self._find(), []) + + def test_intern_section_ignored(self): + """D-Bus-written [.intern.global-dns] groups do not match.""" + path = os.path.join(self.dirs[2], "intern.conf") + self._write(path, "[.intern.global-dns]\nsearches=example.com\n") + self.assertEqual(self._find(), []) + + def test_shadowed_snippet_ignored(self): + """A same-named snippet in a later dir hides the earlier one.""" + self._write( + os.path.join(self.dirs[0], "dns.conf"), "[global-dns]\noptions=no-aaaa\n" + ) + self._write(os.path.join(self.dirs[2], "dns.conf"), "[main]\n") + self.assertEqual(self._find(), []) + + def test_non_utf8_content(self): + """Non-UTF-8 bytes in a config file do not break matching.""" + path = os.path.join(self.dirs[2], "dns.conf") + self._write(path, b"# \xff\n[global-dns]\n", "wb") + self.assertEqual(self._find(), [path]) + + def test_missing_files_skipped(self): + """A missing config file is skipped.""" + os.remove(self.config_file) + self.assertEqual(self._find(), []) + + def test_run_rejects_dns_resolver_with_global_dns(self): + """run fails before apply when global DNS is configured.""" + self._write(self.config_file, "[global-dns]\n") + with self.assertRaises(_FailJson): + self._run({"dns-resolver": {"config": {}}}) + self.assertFalse(self.libnmstate.apply.called) + + def test_run_applies_dns_resolver_without_global_dns(self): + """run applies dns-resolver when no global DNS is configured.""" + self._run({"dns-resolver": {"config": {}}}) + self.libnmstate.apply.assert_called_once_with({"dns-resolver": {"config": {}}}) + + def test_run_ignores_global_dns_without_dns_resolver(self): + """run applies states without dns-resolver regardless of global DNS.""" + self._write(self.config_file, "[global-dns]\n") + self._run({"interfaces": []}) + self.assertTrue(self.libnmstate.apply.called) + + +if __name__ == "__main__": + unittest.main()