From 7352457e7b9088ed0ae40dacf626bf677bc850fc Mon Sep 17 00:00:00 2001 From: Sam Doran Date: Tue, 27 Oct 2020 17:30:54 -0400 Subject: [PATCH] hostname - add macOS (#54439) * Add DarwinStrategy class and integration tests macOS has three seprate hostname params that need to be set. One of those params, LocalHostName, has more stringent requirements than the other two, which accept special characters and spaces. Create a method to scrub the hostname to ensure it works well with the system requirements. * Update documentation * Account for virtualization type returned on Azure Pipelines * Do not be dependent on order of self.name_types Use the scrubbed name when the name type is LocalHostName --- .../322214-hostname-macos-support.yml | 2 + lib/ansible/modules/hostname.py | 160 +++++++++++++++++- test/integration/targets/hostname/aliases | 2 - .../targets/hostname/tasks/MacOSX.yml | 52 ++++++ .../targets/hostname/tasks/main.yml | 89 +--------- .../hostname/tasks/test_check_mode.yml | 50 ++++++ .../targets/hostname/tasks/test_normal.yml | 24 +++ 7 files changed, 289 insertions(+), 90 deletions(-) create mode 100644 changelogs/fragments/322214-hostname-macos-support.yml create mode 100644 test/integration/targets/hostname/tasks/MacOSX.yml create mode 100644 test/integration/targets/hostname/tasks/test_check_mode.yml create mode 100644 test/integration/targets/hostname/tasks/test_normal.yml diff --git a/changelogs/fragments/322214-hostname-macos-support.yml b/changelogs/fragments/322214-hostname-macos-support.yml new file mode 100644 index 00000000000..42ffd6b7188 --- /dev/null +++ b/changelogs/fragments/322214-hostname-macos-support.yml @@ -0,0 +1,2 @@ +bugfixes: + - hostname - add macOS support (https://github.com/ansible/ansible/pull/54439) diff --git a/lib/ansible/modules/hostname.py b/lib/ansible/modules/hostname.py index 7df647a4e5f..2a13d2ef5a3 100644 --- a/lib/ansible/modules/hostname.py +++ b/lib/ansible/modules/hostname.py @@ -18,20 +18,25 @@ version_added: "1.4" short_description: Manage hostname requirements: [ hostname ] description: - - Set system's hostname, supports most OSs/Distributions, including those using systemd. - - Note, this module does *NOT* modify C(/etc/hosts). You need to modify it yourself using other modules like template or replace. - - Windows, macOS, HP-UX and AIX are not currently supported. + - Set system's hostname. Supports most OSs/Distributions including those using C(systemd). + - Windows, HP-UX, and AIX are not currently supported. +notes: + - This module does B(NOT) modify C(/etc/hosts). You need to modify it yourself using other modules like M(template) or M(replace). + - On macOS, this module uses C(scutil) to set C(HostName), C(ComputerName), and C(LocalHostName). Since C(LocalHostName) + cannot contain spaces or most special characters, this module will replace characters when setting C(LocalHostName). options: name: description: - Name of the host + - If the value is a fully qualified domain name that does not resolve from the given host, + this will cause the module to hang for a few seconds while waiting for the name resolution attempt to timeout. type: str required: true use: description: - Which strategy to use to update the hostname. - If not set we try to autodetect, but this can be problematic, particularly with containers as they can present misleading information. - choices: ['generic', 'debian', 'sles', 'redhat', 'alpine', 'systemd', 'openrc', 'openbsd', 'solaris', 'freebsd'] + choices: ['alpine', 'debian', 'freebsd', 'generic', 'macos', 'macosx', 'darwin', 'openbsd', 'openrc', 'redhat', 'sles', 'solaris', 'systemd'] type: str version_added: '2.9' ''' @@ -40,6 +45,11 @@ EXAMPLES = ''' - name: Set a hostname hostname: name: web01 + +- name: Set a hostname specifying strategy + hostname: + name: web01 + strategy: systemd ''' import os @@ -54,10 +64,24 @@ from ansible.module_utils.basic import ( ) from ansible.module_utils.common.sys_info import get_platform_subclass from ansible.module_utils.facts.system.service_mgr import ServiceMgrFactCollector -from ansible.module_utils._text import to_native - -STRATS = {'generic': 'Generic', 'debian': 'Debian', 'sles': 'SLES', 'redhat': 'RedHat', 'alpine': 'Alpine', - 'systemd': 'Systemd', 'openrc': 'OpenRC', 'openbsd': 'OpenBSD', 'solaris': 'Solaris', 'freebsd': 'FreeBSD'} +from ansible.module_utils._text import to_native, to_text +from ansible.module_utils.six import PY3, text_type + +STRATS = { + 'alpine': 'Alpine', + 'debian': 'Debian', + 'freebsd': 'FreeBSD', + 'generic': 'Generic', + 'macos': 'Darwin', + 'macosx': 'Darwin', + 'darwin': 'Darwin', + 'openbsd': 'OpenBSD', + 'openrc': 'OpenRC', + 'redhat': 'RedHat', + 'sles': 'SLES', + 'solaris': 'Solaris', + 'systemd': 'Systemd', +} class UnimplementedStrategy(object): @@ -580,6 +604,116 @@ class FreeBSDStrategy(GenericStrategy): f.close() +class DarwinStrategy(GenericStrategy): + """ + This is a macOS hostname manipulation strategy class. It uses + /usr/sbin/scutil to set ComputerName, HostName, and LocalHostName. + + HostName corresponds to what most platforms consider to be hostname. + It controls the name used on the command line and SSH. + + However, macOS also has LocalHostName and ComputerName settings. + LocalHostName controls the Bonjour/ZeroConf name, used by services + like AirDrop. This class implements a method, _scrub_hostname(), that mimics + the transformations macOS makes on hostnames when enterened in the Sharing + preference pane. It replaces spaces with dashes and removes all special + characters. + + ComputerName is the name used for user-facing GUI services, like the + System Preferences/Sharing pane and when users connect to the Mac over the network. + """ + + def __init__(self, module): + super(DarwinStrategy, self).__init__(module) + self.scutil = self.module.get_bin_path('scutil', True) + self.name_types = ('HostName', 'ComputerName', 'LocalHostName') + self.scrubbed_name = self._scrub_hostname(self.module.params['name']) + + def _make_translation(self, replace_chars, replacement_chars, delete_chars): + if PY3: + return str.maketrans(replace_chars, replacement_chars, delete_chars) + + if not isinstance(replace_chars, text_type) or not isinstance(replacement_chars, text_type): + raise ValueError('replace_chars and replacement_chars must both be strings') + if len(replace_chars) != len(replacement_chars): + raise ValueError('replacement_chars must be the same length as replace_chars') + + table = dict(zip((ord(c) for c in replace_chars), replacement_chars)) + for char in delete_chars: + table[ord(char)] = None + + return table + + def _scrub_hostname(self, name): + """ + LocalHostName only accepts valid DNS characters while HostName and ComputerName + accept a much wider range of characters. This function aims to mimic how macOS + translates a friendly name to the LocalHostName. + """ + + # Replace all these characters with a single dash + name = to_text(name) + replace_chars = u'\'"~`!@#$%^&*(){}[]/=?+\\|-_ ' + delete_chars = u".'" + table = self._make_translation(replace_chars, u'-' * len(replace_chars), delete_chars) + name = name.translate(table) + + # Replace multiple dashes with a single dash + while '-' * 2 in name: + name = name.replace('-' * 2, '') + + name = name.rstrip('-') + return name + + def get_current_hostname(self): + cmd = [self.scutil, '--get', 'HostName'] + rc, out, err = self.module.run_command(cmd) + if rc != 0 and 'HostName: not set' not in err: + self.module.fail_json(msg="Failed to get current hostname rc=%d, out=%s, err=%s" % (rc, out, err)) + + return to_native(out).strip() + + def get_permanent_hostname(self): + cmd = [self.scutil, '--get', 'ComputerName'] + rc, out, err = self.module.run_command(cmd) + if rc != 0: + self.module.fail_json(msg="Failed to get permanent hostname rc=%d, out=%s, err=%s" % (rc, out, err)) + + return to_native(out).strip() + + def set_permanent_hostname(self, name): + for hostname_type in self.name_types: + cmd = [self.scutil, '--set', hostname_type] + if hostname_type == 'LocalHostName': + cmd.append(to_native(self.scrubbed_name)) + else: + cmd.append(to_native(name)) + rc, out, err = self.module.run_command(cmd) + if rc != 0: + self.module.fail_json(msg="Failed to set {3} to '{2}': {0} {1}".format(to_native(out), to_native(err), to_native(name), hostname_type)) + + def set_current_hostname(self, name): + pass + + def update_current_hostname(self): + pass + + def update_permanent_hostname(self): + name = self.module.params['name'] + + # Get all the current host name values in the order of self.name_types + all_names = tuple(self.module.run_command([self.scutil, '--get', name_type])[1].strip() for name_type in self.name_types) + + # Get the expected host name values based on the order in self.name_types + expected_names = tuple(self.scrubbed_name if n == 'LocalHostName' else name for n in self.name_types) + + # Ensure all three names are updated + if all_names != expected_names: + if not self.module.check_mode: + self.set_permanent_hostname(name) + self.changed = True + + class FedoraHostname(Hostname): platform = 'Linux' distribution = 'Fedora' @@ -822,6 +956,12 @@ class NeonHostname(Hostname): strategy_class = DebianStrategy +class DarwinHostname(Hostname): + platform = 'Darwin' + distribution = None + strategy_class = DarwinStrategy + + class OsmcHostname(Hostname): platform = 'Linux' distribution = 'Osmc' @@ -867,7 +1007,11 @@ def main(): name_before = current_hostname elif name != permanent_hostname: name_before = permanent_hostname + else: + name_before = permanent_hostname + # NOTE: socket.getfqdn() calls gethostbyaddr(socket.gethostname()), which can be + # slow to return if the name does not resolve correctly. kw = dict(changed=changed, name=name, ansible_facts=dict(ansible_hostname=name.split('.')[0], ansible_nodename=name, diff --git a/test/integration/targets/hostname/aliases b/test/integration/targets/hostname/aliases index ef782ac73fa..c552d611499 100644 --- a/test/integration/targets/hostname/aliases +++ b/test/integration/targets/hostname/aliases @@ -1,5 +1,3 @@ shippable/posix/group1 destructive skip/aix # currently unsupported by hostname module -skip/osx # same, see #54439, #32214 -skip/macos diff --git a/test/integration/targets/hostname/tasks/MacOSX.yml b/test/integration/targets/hostname/tasks/MacOSX.yml new file mode 100644 index 00000000000..912ced707b6 --- /dev/null +++ b/test/integration/targets/hostname/tasks/MacOSX.yml @@ -0,0 +1,52 @@ +- name: macOS | Set hostname + hostname: + name: bugs.acme.example.com + +# These tasks can be changed to a loop once https://github.com/ansible/ansible/issues/71031 +# is fixed +- name: macOS | Set hostname specifiying macos strategy + hostname: + name: bugs.acme.example.com + use: macos + +- name: macOS | Set hostname specifiying macosx strategy + hostname: + name: bugs.acme.example.com + use: macosx + +- name: macOS | Set hostname specifiying darwin strategy + hostname: + name: bugs.acme.example.com + use: darwin + +- name: macOS | Get macOS hostname values + command: scutil --get {{ item }} + loop: + - HostName + - ComputerName + - LocalHostName + register: macos_scutil + ignore_errors: yes + +- name: macOS | Ensure all hostname values were set correctly + assert: + that: + - "['bugs.acme.example.com', 'bugs.acme.example.com', 'bugsacmeexamplecom'] == macos_scutil.results | map(attribute='stdout') | list" + +- name: macOS | Set to a hostname using spaces and punctuation + hostname: + name: The Dude's Computer + +- name: macOS | Get macOS hostname values + command: scutil --get {{ item }} + loop: + - HostName + - ComputerName + - LocalHostName + register: macos_scutil_complex + ignore_errors: yes + +- name: macOS | Ensure all hostname values were set correctly + assert: + that: + - "['The Dude\\'s Computer', 'The Dude\\'s Computer', 'The-Dudes-Computer'] == (macos_scutil_complex.results | map(attribute='stdout') | list)" diff --git a/test/integration/targets/hostname/tasks/main.yml b/test/integration/targets/hostname/tasks/main.yml index 3f6ec4b49f7..8a9e34bde1d 100644 --- a/test/integration/targets/hostname/tasks/main.yml +++ b/test/integration/targets/hostname/tasks/main.yml @@ -16,80 +16,8 @@ command: hostname register: original - - name: Run hostname module in check_mode - hostname: - name: crocodile.ansible.test.doesthiswork.net.example.com - check_mode: true - register: hn1 - - - name: Get current hostname again - command: hostname - register: after_hn - - - assert: - that: - - hn1 is changed - - original.stdout == after_hn.stdout - - - when: _hostname_file is defined and _hostname_file - block: - - name: See if current hostname file exists - stat: - path: "{{ _hostname_file }}" - register: hn_stat - - - name: Move the current hostname file if it exists - command: mv {{ _hostname_file }} {{ _hostname_file }}.orig - when: hn_stat.stat.exists - - - name: Run hostname module in check_mode - hostname: - name: crocodile.ansible.test.doesthiswork.net.example.com - check_mode: true - register: hn - - - stat: - path: /etc/rc.conf.d/hostname - register: hn_stat_checkmode - - - assert: - that: - # TODO: This is a legitimate bug and will be fixed in another PR. - # - not hn_stat_checkmode.stat.exists - - hn is changed - - - name: Get hostname again - command: hostname - register: current_after_cm - - - assert: - that: - - original.stdout == current_after_cm.stdout - - - name: Run hostname module for real now - hostname: - name: crocodile.ansible.test.doesthiswork.net.example.com - register: hn2 - - - name: Get hostname - command: hostname - register: current_after_hn2 - - - name: Run hostname again to ensure it is idempotent - hostname: - name: crocodile.ansible.test.doesthiswork.net.example.com - register: hnidem - - - name: Get hostname - command: hostname - register: current_after_hnidem - - - assert: - that: - - hn2 is changed - - hnidem is not changed - - current_after_hn2.stdout == 'crocodile.ansible.test.doesthiswork.net.example.com' - - current_after_hn2.stdout == current_after_hnidem.stdout + - import_tasks: test_check_mode.yml + - import_tasks: test_normal.yml - name: Include distribution specific tasks include_tasks: @@ -98,24 +26,25 @@ files: - "{{ ansible_facts.distribution }}.yml" - default.yml + always: # Reset back to original hostname - name: Move back original file if it existed command: mv -f {{ _hostname_file }}.orig {{ _hostname_file }} - when: hn_stat is defined and hn_stat.stat.exists + when: hn_stat.stat.exists | default(False) - name: Delete the file if it never existed file: path: "{{ _hostname_file }}" state: absent - when: hn_stat is defined and not hn_stat.stat.exists + when: not hn_stat.stat.exists | default(True) - # Reset back to original hostname - - hostname: + - name: Reset back to original hostname + hostname: name: "{{ original.stdout }}" register: revert - # And make sure we really do - - assert: + - name: Ensure original hostname was reset + assert: that: - revert is changed diff --git a/test/integration/targets/hostname/tasks/test_check_mode.yml b/test/integration/targets/hostname/tasks/test_check_mode.yml new file mode 100644 index 00000000000..9ba1d65c720 --- /dev/null +++ b/test/integration/targets/hostname/tasks/test_check_mode.yml @@ -0,0 +1,50 @@ +- name: Run hostname module in check_mode + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: hn1 + +- name: Get current hostname again + command: hostname + register: after_hn + +- name: Ensure hostname changed properly + assert: + that: + - hn1 is changed + - original.stdout == after_hn.stdout + +- when: _hostname_file is defined and _hostname_file + block: + - name: See if current hostname file exists + stat: + path: "{{ _hostname_file }}" + register: hn_stat + + - name: Move the current hostname file if it exists + command: mv {{ _hostname_file }} {{ _hostname_file }}.orig + when: hn_stat.stat.exists + + - name: Run hostname module in check_mode + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + check_mode: true + register: hn + + - stat: + path: /etc/rc.conf.d/hostname + register: hn_stat_checkmode + + - assert: + that: + # TODO: This is a legitimate bug and will be fixed in another PR. + # - not hn_stat_checkmode.stat.exists + - hn is changed + + - name: Get hostname again + command: hostname + register: current_after_cm + + - assert: + that: + - original.stdout == current_after_cm.stdout diff --git a/test/integration/targets/hostname/tasks/test_normal.yml b/test/integration/targets/hostname/tasks/test_normal.yml new file mode 100644 index 00000000000..a40d96e97b2 --- /dev/null +++ b/test/integration/targets/hostname/tasks/test_normal.yml @@ -0,0 +1,24 @@ +- name: Run hostname module for real now + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + register: hn2 + +- name: Get hostname + command: hostname + register: current_after_hn2 + +- name: Run hostname again to ensure it does not change + hostname: + name: crocodile.ansible.test.doesthiswork.net.example.com + register: hn3 + +- name: Get hostname + command: hostname + register: current_after_hn3 + +- assert: + that: + - hn2 is changed + - hn3 is not changed + - current_after_hn2.stdout == 'crocodile.ansible.test.doesthiswork.net.example.com' + - current_after_hn2.stdout == current_after_hn2.stdout