From 2405861a9e73a9acabf5b884573325ac459f8a6d Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Thu, 17 Sep 2015 14:42:16 +0530 Subject: [PATCH 1/6] Use ',' instead of ':' or ';' to separate host patterns The earlier-recommended "pat1:pat2:pat3[x:y]" notation doesn't work well with IPv6 addresses, so we recommend ',' as a separator instead. We know that commas can't occur within a pattern, so we can just split on it. We still have to accept the "foo:bar" notation because it's so commonly used, but we issue a deprecation warning for it. Fixes #12296 Closes #12404 Closes #12329 --- lib/ansible/inventory/__init__.py | 80 +++++++++++++++++++++---------- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 85290cf8a74..079243b0cd8 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -149,23 +149,6 @@ class Inventory(object): results.append(item) return results - def _split_pattern(self, pattern): - """ - takes e.g. "webservers[0:5]:dbservers:others" - and returns ["webservers[0:5]", "dbservers", "others"] - """ - - term = re.compile( - r'''(?: # We want to match something comprising: - [^:\[\]] # (anything other than ':', '[', or ']' - | # ...or... - \[[^\]]*\] # a single complete bracketed expression) - )* # repeated as many times as possible - ''', re.X - ) - - return [x for x in term.findall(pattern) if x] - def get_hosts(self, pattern="all", ignore_limits_and_restrictions=False): """ Takes a pattern or list of patterns and returns a list of matching @@ -173,14 +156,6 @@ class Inventory(object): or applied subsets """ - # Enumerate all hosts matching the given pattern (which may be - # either a list of patterns or a string like 'pat1:pat2'). - if isinstance(pattern, list): - pattern = ':'.join(pattern) - - if ';' in pattern or ',' in pattern: - display.deprecated("Use ':' instead of ',' or ';' to separate host patterns", version=2.0, removed=True) - patterns = self._split_pattern(pattern) hosts = self._evaluate_patterns(patterns) @@ -197,6 +172,59 @@ class Inventory(object): return hosts + def _split_pattern(self, pattern): + """ + Takes a string containing host patterns separated by commas (or a list + thereof) and returns a list of single patterns (which may not contain + commas). Whitespace is ignored. + + Also accepts ':' as a separator for backwards compatibility, but it is + not recommended due to the conflict with IPv6 addresses and host ranges. + + Example: 'a,b[1], c[2:3] , d' -> ['a', 'b[1]', 'c[2:3]', 'd'] + """ + + if isinstance(pattern, list): + pattern = ','.join(pattern) + + patterns = [] + + if ';' in pattern: + display.deprecated("Use ',' instead of ':' or ';' to separate host patterns", version=2.0, removed=True) + + # If it's got commas in it, we'll treat it as a straightforward + # comma-separated list of patterns. + + elif ',' in pattern: + patterns = re.split('\s*,\s*', pattern) + + # 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: + 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: + patterns = re.findall( + r'''(?: # We want to match something comprising: + [^\s:\[\]] # (anything other than whitespace or ':[]' + | # ...or... + \[[^\]]*\] # a single complete bracketed expression) + )+ # occurring once or more + ''', pattern, re.X + ) + + if len(patterns) > 1: + display.deprecated("Use ',' instead of ':' or ';' to separate host patterns", version=2.0) + + return [p.strip() for p in patterns] + def _evaluate_patterns(self, patterns): """ Takes a list of patterns and returns a list of matching host names, @@ -249,7 +277,7 @@ class Inventory(object): The pattern may be: 1. A regex starting with ~, e.g. '~[abc]*' - 2. A shell glob pattern with ?/*/[chars]/[!chars], e.g. 'foo' + 2. A shell glob pattern with ?/*/[chars]/[!chars], e.g. 'foo*' 3. An ordinary word that matches itself only, e.g. 'foo' The pattern is matched using the following rules: From abd006657bc302db4f1979610fe738bba7aa6ce4 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Thu, 17 Sep 2015 19:32:14 +0530 Subject: [PATCH 2/6] Add test/units/inventory with a few _split_pattern tests There were no inventory-specific unit tests earlier, so we add a new directory for them with some initial low-level tests of _split_pattern with various valid and deprecated pattern strings. --- test/units/inventory/__init__.py | 21 ++++++++++ test/units/inventory/test_inventory.py | 58 ++++++++++++++++++++++++++ 2 files changed, 79 insertions(+) create mode 100644 test/units/inventory/__init__.py create mode 100644 test/units/inventory/test_inventory.py diff --git a/test/units/inventory/__init__.py b/test/units/inventory/__init__.py new file mode 100644 index 00000000000..785fc459921 --- /dev/null +++ b/test/units/inventory/__init__.py @@ -0,0 +1,21 @@ +# (c) 2012-2014, Michael DeHaan +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + diff --git a/test/units/inventory/test_inventory.py b/test/units/inventory/test_inventory.py new file mode 100644 index 00000000000..3869e15b587 --- /dev/null +++ b/test/units/inventory/test_inventory.py @@ -0,0 +1,58 @@ +# Copyright 2015 Abhijit Menon-Sen +# +# This file is part of Ansible +# +# Ansible is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# Ansible is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with Ansible. If not, see . + +# Make coding more python3-ish +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible.compat.tests import unittest +from ansible.compat.tests.mock import patch, MagicMock + +from ansible.errors import AnsibleError, AnsibleParserError +from ansible.inventory import Inventory +from ansible.vars import VariableManager + +from units.mock.loader import DictDataLoader + +class TestInventory(unittest.TestCase): + + patterns = { + 'a': ['a'], + 'a, b': ['a', 'b'], + 'a , b': ['a', 'b'], + ' a,b ,c[1:2] ': ['a', 'b', 'c[1:2]'], + '9a01:7f8:191:7701::9': ['9a01:7f8:191:7701::9'], + '9a01:7f8:191:7701::9,9a01:7f8:191:7701::9': ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9'], + '9a01:7f8:191:7701::9,9a01:7f8:191:7701::9,foo': ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9','foo'], + 'foo[1:2]': ['foo[1:2]'], + 'a::b': ['a::b'], + 'a:b': ['a', 'b'], + ' a : b ': ['a', 'b'], + 'foo:bar:baz[1:2]': ['foo', 'bar', 'baz[1:2]'], + } + + def setUp(self): + v = VariableManager() + fake_loader = DictDataLoader({}) + + self.i = Inventory(loader=fake_loader, variable_manager=v, host_list='') + + def test_split_patterns(self): + + for p in self.patterns: + r = self.patterns[p] + self.assertEqual(r, self.i._split_pattern(p)) From b47bc343ea64cbee77c0a499fbc3293427fa8838 Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Thu, 17 Sep 2015 16:34:45 +0530 Subject: [PATCH 3/6] Document , instead of : in intro_patterns, update changelog --- CHANGELOG.md | 6 ++++-- docsite/rst/intro_patterns.rst | 14 +++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e5f282ed41..a8e43330dff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,10 @@ Major Changes: They will retain the value of `None`. To go back to the old behaviour, you can override the `null_representation` setting to an empty string in your config file or by setting the `ANSIBLE_NULL_REPRESENTATION` environment variable. +* Use "pattern1,pattern2" to combine host matching patterns. The use of + ':' as a separator is deprecated (accepted with a warning) because it + conflicts with IPv6 addresses. The undocumented use of ';' as a + separator is no longer supported. * Backslashes used when specifying parameters in jinja2 expressions in YAML dicts sometimes needed to be escaped twice. This has been fixed so that escaping once works. Here's an example of how playbooks need to be modified: @@ -252,8 +256,6 @@ Minor changes: * Many more tests. The new API makes things more testable and we took advantage of it. * big_ip modules now support turning off ssl certificate validation (use only for self-signed certificates). -* Use "pattern1:pattern2" to combine host matching patterns. The undocumented -use of semicolons or commas to combine patterns is no longer supported. * Use ``hosts: groupname[x:y]`` to select a subset of hosts in a group; the ``[x-y]`` range syntax is no longer supported. Note that ``[0:1]`` matches two hosts, i.e. the range is inclusive of its endpoints. diff --git a/docsite/rst/intro_patterns.rst b/docsite/rst/intro_patterns.rst index 8315c09b512..9238d92869e 100644 --- a/docsite/rst/intro_patterns.rst +++ b/docsite/rst/intro_patterns.rst @@ -27,7 +27,7 @@ The following patterns are equivalent and target all hosts in the inventory:: It is also possible to address a specific host or set of hosts by name:: one.example.com - one.example.com:two.example.com + one.example.com, two.example.com 192.168.1.50 192.168.1.* @@ -35,20 +35,20 @@ The following patterns address one or more groups. Groups separated by a colon This means the host may be in either one group or the other:: webservers - webservers:dbservers + webservers,dbservers You can exclude groups as well, for instance, all machines must be in the group webservers but not in the group phoenix:: - webservers:!phoenix + webservers,!phoenix You can also specify the intersection of two groups. This would mean the hosts must be in the group webservers and the host must also be in the group staging:: - webservers:&staging + webservers,&staging You can do combinations:: - webservers:dbservers:&staging:!phoenix + webservers,dbservers,&staging,!phoenix The above configuration means "all machines in the groups 'webservers' and 'dbservers' are to be managed if they are in the group 'staging' also, but the machines are not to be managed if they are in the group 'phoenix' ... whew! @@ -56,7 +56,7 @@ the group 'staging' also, but the machines are not to be managed if they are in You can also use variables if you want to pass some group specifiers via the "-e" argument to ansible-playbook, but this is uncommonly used:: - webservers:!{{excluded}}:&{{required}} + webservers,!{{excluded}},&{{required}} You also don't have to manage by strictly defined groups. Individual host names, IPs and groups, can also be referenced using wildcards:: @@ -66,7 +66,7 @@ wildcards:: It's also ok to mix wildcard patterns and groups at the same time:: - one*.com:dbservers + one*.com,dbservers You can select a host or subset of hosts from a group by their position. For example, given the following group:: From 349eec7855549ca2c458c222ae10406193a25cae Mon Sep 17 00:00:00 2001 From: Abhijit Menon-Sen Date: Thu, 17 Sep 2015 19:23:38 +0530 Subject: [PATCH 4/6] Fix missing colon (typo) in IPv6 pattern --- lib/ansible/parsing/utils/addresses.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ansible/parsing/utils/addresses.py b/lib/ansible/parsing/utils/addresses.py index 71675769d27..294025e3ddd 100644 --- a/lib/ansible/parsing/utils/addresses.py +++ b/lib/ansible/parsing/utils/addresses.py @@ -122,7 +122,7 @@ patterns = { r'''^ (?:{0}:){{7}}{0}| # uncompressed: 1:2:3:4:5:6:7:8 (?:{0}:){{1,6}}:| # compressed variants, which are all - (?:{0}:)(?:{0}){{1,6}}| # a::b for various lengths of a,b + (?:{0}:)(?::{0}){{1,6}}| # a::b for various lengths of a,b (?:{0}:){{2}}(?::{0}){{1,5}}| (?:{0}:){{3}}(?::{0}){{1,4}}| (?:{0}:){{4}}(?::{0}){{1,3}}| From 01ae60d5240370359e97a8a79759a3c80eea46a0 Mon Sep 17 00:00:00 2001 From: Victor Salgado Date: Thu, 17 Sep 2015 13:50:40 -0300 Subject: [PATCH 5/6] Add more tests for _split_pattern for when the input is a list --- test/units/inventory/test_inventory.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/test/units/inventory/test_inventory.py b/test/units/inventory/test_inventory.py index 3869e15b587..e397143390b 100644 --- a/test/units/inventory/test_inventory.py +++ b/test/units/inventory/test_inventory.py @@ -44,6 +44,14 @@ class TestInventory(unittest.TestCase): ' a : b ': ['a', 'b'], 'foo:bar:baz[1:2]': ['foo', 'bar', 'baz[1:2]'], } + pattern_lists = [ + [['a'], ['a']], + [['a', 'b'], ['a', 'b']], + [['a, b'], ['a', 'b']], + [['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9,foo'], + ['9a01:7f8:191:7701::9', '9a01:7f8:191:7701::9','foo']] + ] + def setUp(self): v = VariableManager() @@ -56,3 +64,6 @@ class TestInventory(unittest.TestCase): for p in self.patterns: r = self.patterns[p] self.assertEqual(r, self.i._split_pattern(p)) + + for p, r in self.pattern_lists: + self.assertEqual(r, self.i._split_pattern(p)) From 14fefebaad0d5a6fc70cb913b138da78345818f9 Mon Sep 17 00:00:00 2001 From: Victor Salgado Date: Thu, 17 Sep 2015 13:52:54 -0300 Subject: [PATCH 6/6] Modify _split_pattern to use map when working with list input --- lib/ansible/inventory/__init__.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/ansible/inventory/__init__.py b/lib/ansible/inventory/__init__.py index 079243b0cd8..643f12d8fd3 100644 --- a/lib/ansible/inventory/__init__.py +++ b/lib/ansible/inventory/__init__.py @@ -24,6 +24,7 @@ import os import sys import re import stat +import itertools from ansible import constants as C from ansible.errors import AnsibleError @@ -185,9 +186,7 @@ class Inventory(object): """ if isinstance(pattern, list): - pattern = ','.join(pattern) - - patterns = [] + return list(itertools.chain(*map(self._split_pattern, pattern))) if ';' in pattern: display.deprecated("Use ',' instead of ':' or ';' to separate host patterns", version=2.0, removed=True)