From 75e94e0cba538c9ed532374b219c45e91fd89db8 Mon Sep 17 00:00:00 2001 From: Brian Coca Date: Mon, 21 Dec 2015 13:06:48 -0500 Subject: [PATCH] allow for non standard hostnames * Changed parse_addresses to throw exceptions instead of passing None * Switched callers to trap and pass through the original values. * Added very verbose notice * Look at deprecating this and possibly validate at plugin instead fixes #13608 --- lib/ansible/inventory/__init__.py | 21 ++++++++++++--------- lib/ansible/inventory/ini.py | 11 +++++++---- lib/ansible/parsing/utils/addresses.py | 22 +++++++++++----------- lib/ansible/plugins/action/add_host.py | 10 +++++++--- test/units/parsing/test_addresses.py | 14 ++++++++++++-- 5 files changed, 49 insertions(+), 29 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 95e193f381a..095118e50eb 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -109,7 +109,12 @@ class Inventory(object): pass elif isinstance(host_list, list): for h in host_list: - (host, port) = parse_address(h, allow_ranges=False) + try: + (host, port) = parse_address(h, allow_ranges=False) + except AnsibleError as e: + display.vvv("Unable to parse address from hostname, leaving unchanged: %s" % to_string(e)) + host = h + port = None all.add_host(Host(host, port)) elif self._loader.path_exists(host_list): #TODO: switch this to a plugin loader and a 'condition' per plugin on which it should be tried, restoring 'inventory pllugins' @@ -228,15 +233,13 @@ class Inventory(object): # If it doesn't, it could still be a single pattern. This accounts for # non-separator uses of colons: IPv6 addresses and [x:y] host ranges. else: - (base, port) = parse_address(pattern, allow_ranges=True) - if base: + try: + (base, port) = parse_address(pattern, allow_ranges=True) patterns = [pattern] - - # The only other case we accept is a ':'-separated list of patterns. - # This mishandles IPv6 addresses, and is retained only for backwards - # compatibility. - - else: + except: + # The only other case we accept is a ':'-separated list of patterns. + # This mishandles IPv6 addresses, and is retained only for backwards + # compatibility. patterns = re.findall( r'''(?: # We want to match something comprising: [^\s:\[\]] # (anything other than whitespace or ':[]' diff --git a/lib/ansible/inventory/ini.py b/lib/ansible/inventory/ini.py index 537fde1ef9e..9224ef2d23d 100644 --- a/lib/ansible/inventory/ini.py +++ b/lib/ansible/inventory/ini.py @@ -23,7 +23,7 @@ import ast import re from ansible import constants as C -from ansible.errors import AnsibleError +from ansible.errors import AnsibleError, AnsibleParserError from ansible.inventory.host import Host from ansible.inventory.group import Group from ansible.inventory.expand_hosts import detect_range @@ -264,9 +264,12 @@ class InventoryParser(object): # Can the given hostpattern be parsed as a host with an optional port # specification? - (pattern, port) = parse_address(hostpattern, allow_ranges=True) - if not pattern: - self._raise_error("Can't parse '%s' as host[:port]" % hostpattern) + try: + (pattern, port) = parse_address(hostpattern, allow_ranges=True) + except: + # not a recognizable host pattern + pattern = hostpattern + port = None # Once we have separated the pattern, we expand it into list of one or # more hostnames, depending on whether it contains any [x:y] ranges. diff --git a/lib/ansible/parsing/utils/addresses.py b/lib/ansible/parsing/utils/addresses.py index 387f05c627f..ebfd850ac6a 100644 --- a/lib/ansible/parsing/utils/addresses.py +++ b/lib/ansible/parsing/utils/addresses.py @@ -20,6 +20,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import re +from ansible.errors import AnsibleParserError, AnsibleError # Components that match a numeric or alphanumeric begin:end or begin:end:step # range expression inside square brackets. @@ -162,6 +163,7 @@ patterns = { $ '''.format(label=label), re.X|re.I|re.UNICODE ), + } def parse_address(address, allow_ranges=False): @@ -183,8 +185,8 @@ def parse_address(address, allow_ranges=False): # First, we extract the port number if one is specified. port = None - for type in ['bracketed_hostport', 'hostport']: - m = patterns[type].match(address) + for matching in ['bracketed_hostport', 'hostport']: + m = patterns[matching].match(address) if m: (address, port) = m.groups() port = int(port) @@ -194,22 +196,20 @@ def parse_address(address, allow_ranges=False): # numeric ranges, or a hostname with alphanumeric ranges. host = None - for type in ['ipv4', 'ipv6', 'hostname']: - m = patterns[type].match(address) + for matching in ['ipv4', 'ipv6', 'hostname']: + m = patterns[matching].match(address) if m: host = address continue # If it isn't any of the above, we don't understand it. - if not host: - return (None, None) - - # If we get to this point, we know that any included ranges are valid. If - # the caller is prepared to handle them, all is well. Otherwise we treat - # it as a parse failure. + raise AnsibleError("Not a valid network hostname: %s" % address) + # If we get to this point, we know that any included ranges are valid. + # If the caller is prepared to handle them, all is well. + # Otherwise we treat it as a parse failure. if not allow_ranges and '[' in host: - return (None, None) + raise AnsibleParserError("Detected range in host but was asked to ignore ranges") return (host, port) diff --git a/lib/ansible/plugins/action/add_host.py b/lib/ansible/plugins/action/add_host.py index 4bf43f14009..b3aec20437e 100644 --- a/lib/ansible/plugins/action/add_host.py +++ b/lib/ansible/plugins/action/add_host.py @@ -53,9 +53,13 @@ class ActionModule(ActionBase): new_name = self._task.args.get('name', self._task.args.get('hostname', None)) display.vv("creating host via 'add_host': hostname=%s" % new_name) - name, port = parse_address(new_name, allow_ranges=False) - if not name: - raise AnsibleError("Invalid inventory hostname: %s" % new_name) + try: + name, port = parse_address(new_name, allow_ranges=False) + except: + # not a parsable hostname, but might still be usable + name = new_name + port = None + if port: self._task.args['ansible_ssh_port'] = port diff --git a/test/units/parsing/test_addresses.py b/test/units/parsing/test_addresses.py index 870cbb0a14a..a688d0253bd 100644 --- a/test/units/parsing/test_addresses.py +++ b/test/units/parsing/test_addresses.py @@ -71,7 +71,12 @@ class TestParseAddress(unittest.TestCase): for t in self.tests: test = self.tests[t] - (host, port) = parse_address(t) + try: + (host, port) = parse_address(t) + except: + host = None + port = None + assert host == test[0] assert port == test[1] @@ -79,6 +84,11 @@ class TestParseAddress(unittest.TestCase): for t in self.range_tests: test = self.range_tests[t] - (host, port) = parse_address(t, allow_ranges=True) + try: + (host, port) = parse_address(t, allow_ranges=True) + except: + host = None + port = None + assert host == test[0] assert port == test[1]